fix: allow self-imports in v2 addons - #683
Conversation
736cb5d to
d3453b2
Compare
04baec7 to
78435cb
Compare
|
This is a good direction. I agree we need a custom webpack plugin here, so we can use it to make ember-auto-import match the resolving behaviors we have under vite. I had also started a similar branch, not to fix this particular bug, but to fix #685. I'll try out this branch and perhaps it can become the base for both. |
There are two bugs addressed in #603, the first one had good test coverage and a good fix. But the second one -- quotes in dependency requests breaking EAI_DISCOVERED_EXTERNALS -- wasn't really being tested and didn't have a correct fix, which was only noticed incidentally while looking at #683. This adds test coverage and changes the implementation to fix.
|
is this fixed by v2.12.1 ? |
|
No, that was a different bug found on the course of working on this one. |
|
Hey @lifeart 👋 I don't know if you've been following but we've been taking inspiration from this PR for the last few weeks during the Tooling Team office hours to fix issues with ember-auto-import Ed mentioned one of the PRs that we pulled out already, but we just merged a new PR that fixes ember-auto-import for renamed-modules which has introduced a new resolver plugin for webpack: #705 I'm pretty sure that this doesn't encapsulate the title of your PR (i.e. it doesn't fix the self-imports in v2 addons) but you might want to take a pass at incorporating your PR change into the new resolver plugin. It is likely a bit simpler now and would probably need a much smaller, more targeted change 👍 |
797f49f to
e99b450
Compare
| // Allow v2 addons to import from themselves using their package name. | ||
| // Without this, self-imports get externalized because the addon doesn't | ||
| // list itself as its own dependency. | ||
| // See: https://github.com/embroider-build/ember-auto-import/issues/681 |
There was a problem hiding this comment.
this is not a relevant issue link 🤔 this is closed and not related to self-imports
There was a problem hiding this comment.
it's partially related, because in original reported issue there is few failures - in addon compilation side (addon named import inside addon), and app level imports (likely already fixed)
There was a problem hiding this comment.
yes it might be partially related but it's now a stale link that shouldn't be included in a comment because it doesn't actually help people who don't already have the context of how they are related 😂
There was a problem hiding this comment.
agree, changes pushed, comment removed
When a v2 addon imports from itself using its own package name (e.g.
`import { foo } from 'my-addon/utils/foo'` inside my-addon), the
externals handler was incorrectly externalizing it because the addon
doesn't list itself as its own dependency.
Add a check in the externals handler: when a v2 addon's code references
its own package name, let webpack resolve it normally instead of
externalizing.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
e99b450 to
0982ebf
Compare
There was a problem hiding this comment.
Pull request overview
Fixes webpack externals handling in ember-auto-import so v2 addons can import from themselves via their own package name (absolute self-imports), addressing issue #681 and aligning with the resolver infrastructure introduced in #705.
Changes:
- Update the webpack externals handler to avoid externalizing v2 addon self-imports (
name === pkg.name). - Add a new v2 addon test fixture (
addon-self-import) that uses absolute self-imports internally. - Add unit tests asserting both the addon’s main export and a component can resolve self-imports.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/ember-auto-import/ts/webpack.ts | Adds a self-import exception in the v2 addon externals logic so webpack resolves self-imports normally. |
| test-scenarios/v2-addon-test.ts | Adds a new v2 addon fixture plus unit tests that exercise absolute self-imports within the addon. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }; | ||
| addon.pkg.exports = { | ||
| '.': './dist/index.js', | ||
| './*': './dist/*', |
There was a problem hiding this comment.
addon.pkg.exports uses the wildcard mapping './*': './dist/*', which will map subpath imports (like addon-self-import/utils/helper) to extensionless targets (e.g. ./dist/utils/helper). This is inconsistent with the other v2-addon fixtures in this file (which use ./dist/*.js) and can diverge from Node/package-exports expectations. Consider changing the mapping to include the .js extension (or otherwise ensure the target paths match the actual filenames) so the fixture models realistic exports behavior.
| './*': './dist/*', | |
| './*': './dist/*.js', |
There was a problem hiding this comment.
The extensionless pattern ./*: ./dist/* is intentional here — this addon's files are flat .js files that webpack resolves naturally without the explicit extension. The ./*: ./dist/*.js pattern actually causes a different failure (the AMD loader can't find the dependency at runtime) because of how exports resolution interacts with webpack's externals handler. The .js exports pattern with directory/index.js fallback is tested separately in #707's addon-conditional-exports fixture.
Problem
When a v2 addon imports from itself using its own package name (e.g.
import { foo } from 'my-addon/utils/foo'insidemy-addon), the webpack externals handler incorrectly externalizes the import. This happens because the addon doesn't list itself as its own dependency, so the!pkg.hasDependency(name)check treats the self-import as an external module.Solution
Add a targeted check in the externals handler: when a v2 addon's code references its own package name (
name === pkg.name), let webpack resolve it normally instead of externalizing.This is a minimal fix that builds on top of the resolver plugin infrastructure from #705.
Changes
packages/ember-auto-import/ts/webpack.ts: Add self-import check in the v2 addon externals handler, before thehasDependencychecktest-scenarios/v2-addon-test.ts: Addaddon-self-importtest addon that uses absolute self-imports internally, with tests verifying the addon's main export and component both correctly resolve self-importsTest plan
🤖 Generated with Claude Code