Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix autocomplete for wasp package #1921

Merged
merged 5 commits into from
Mar 29, 2024
Merged

Fix autocomplete for wasp package #1921

merged 5 commits into from
Mar 29, 2024

Conversation

sodic
Copy link
Contributor

@sodic sodic commented Mar 26, 2024

Patrially fixes #1839.

This PR enables the IDE to properly autocomplete imports exposed from node_modules/wasp.

Here's how it looks:

autocomplete.mp4

After we merge this, all that's left is hiding the stuff we don't want the user to see: #1922.

@sodic sodic force-pushed the filip-autocomplete-imports branch from 8f58277 to 7635266 Compare March 26, 2024 15:13
@sodic sodic requested a review from infomiho March 26, 2024 15:20
"target": "esnext",
// We're bundling all code in the end so this is the most appropriate option,
// it's also important for autocomplete to work properly.
"moduleResolution": "bundler",
Copy link
Contributor Author

@sodic sodic Mar 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got here by trial and error, but bundler seems the most (if not only) appropriate option for our setup.

By implicit defaults cascading, this got set CommonJs, which is why it failed to recognize ES exports from the wasp package.

The cascading was:

  • target was unset and defaulted to ES3.
  • module was unset and defaulted to CommonJS because the target was ES3.
  • moduleResolution was unset and defaulted to Node because module was CommonJS.
  • moduleResolution: Node can't work with ES imports.

Source for all rules: https://www.typescriptlang.org/tsconfig#module

Copy link
Contributor

@infomiho infomiho Mar 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that this is in the user's tsconfig.json, does this mean that only the new newly created projects will get the benefits of these changes?

Do we also need to update all other templates? Specifically the ones you get when running wasp new:

[1] basic (default)
    Simple starter template with a single page.
[2] todo-ts
    Simple but well-rounded Wasp app implemented with Typescript & full-stack type safety.
[3] saas
    Everything a SaaS needs! Comes with Auth, ChatGPT API, Tailwind, Stripe payments and more. Check out https://opensaas.sh/ for more details.
[4] embeddings
    Comes with code for generating vector embeddings and performing vector similarity search.
[5] ai-generated
    🤖 Describe an app in a couple of sentences and have Wasp AI generate initial code for you. (experimental)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that this is in the user's tsconfig.json, does this mean that only the new newly created projects will get the benefits of these changes?

Yes, any ideas on what we can do here?

Do we also need to update all other templates? Specifically the ones you get when running wasp new:

Apparently yes, nice catch! I thought they all used skeleton... We really do need to change those names 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, those names are a bit tricky. But don't they use skeleton? Except for open-saas?

Copy link
Contributor Author

@sodic sodic Mar 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it out, and unfortunately not (only basic and ai-generated do):

$ for i in {basic,todo-ts,saas,embeddings,ai-generated}; do cabal run wasp-cli new filip-test-$i -- -t $i; done
...
$ rg moduleResolution filip-test* -l
filip-test-basic/tsconfig.json
filip-test-ai-generated/tsconfig.json
$

Copy link
Contributor Author

@sodic sodic Mar 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do I do with that?

Should I do anything now or do we take care of it in the release process: https://github.com/wasp-lang/wasp/tree/main/waspc#typical-release-process.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense to do now, when we still have context about it and it is fresh.

But you can do it another PR if you wish! Or maybe you even have to, for these starters.

Copy link
Contributor Author

@sodic sodic Mar 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sodic sodic changed the title Autocomplete improvements RFC Fix autocomplete for wasp package Mar 26, 2024
@sodic sodic marked this pull request as ready for review March 26, 2024 15:43
@Martinsos
Copy link
Member

Awesome!

@sodic sodic merged commit 25079ce into main Mar 29, 2024
6 checks passed
@sodic sodic deleted the filip-autocomplete-imports branch March 29, 2024 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate and Improve Auto-imports
3 participants