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

Remove node resolve #155

Merged
merged 1 commit into from
Jul 17, 2023
Merged

Remove node resolve #155

merged 1 commit into from
Jul 17, 2023

Conversation

NullVoxPopuli
Copy link
Collaborator

I had thought that this plugin was needed for TS (and other extensions) to resolve itself -- but it's maybe possible that when I came across that tha either:

  • I also typo'd the extensions list to the babel plugin
  • I forgot (somehow) to change the babel extensions
  • Or there was a bug in babel's rollup plugin at the time.

The first and second are way more likely, ofc.

With this change, we can create js, template-only, and ts components and have the template-contents compiled correctly:

I will be following up with smoke tests to automate these checks for us.

Input:
image

Output:
image

@NullVoxPopuli NullVoxPopuli added the bug Something isn't working label Jul 17, 2023
@enspandi
Copy link

enspandi commented Jul 28, 2023

Hey @NullVoxPopuli I think node-resolve is still required...

I get these errors otherwise - even with a completely new v2 addon created with this blueprint and latest ember-cli

[js] [!] RollupError: Could not resolve "../utils/world" from "src/components/hello.ts"
[js] src/components/hello.ts
[js]     at error (/home/andi/source/my-addon/node_modules/.pnpm/[email protected]/node_modules/rollup/dist/shared/rollup.js:279:30)
[js]     at ModuleLoader.handleInvalidResolvedId (/home/andi/source/my-addon/node_modules/.pnpm/[email protected]/node_modules/rollup/dist/shared/rollup.js:24433:24)
[js]     at /home/andi/source/my-addon/node_modules/.pnpm/[email protected]/node_modules/rollup/dist/shared/rollup.js:24395:26
[js] 

These errors seem to occur only with a relative import like '../utils/...'. VSCode is happy though, meaning that it can find the import and follow it to the correct file.


Only code that my test addon has is this:

image

^ When I remove the world(); it can build both modules in the dist


When including nodeResolve({ extensions }), in the rollup config everything works. This was not necessary with rollup-plugin-ts 🤷

@NullVoxPopuli
Copy link
Collaborator Author

NullVoxPopuli commented Jul 28, 2023

Yeah, it's 🙃 we're exploring this option: https://github.com/embroider-build/addon-blueprint/pull/160/files atm

Does that style of configuration (and including extensions in the imports) fix your build as well?

@enspandi
Copy link

Yeah, it's 🙃 we're exploring this option: https://github.com/embroider-build/addon-blueprint/pull/160/files atm

Does that style of configuration (and including extensions in the imports) fix your build as well?

Nice - that works! It feels strange at first but seems to be the way to go 👍

And I also encountered that node-resolve did have some issues, like it didn't properly work with template-only components where I had no component.ts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants