Skip to content

Support type=module v2 addons - #718

Open
NullVoxPopuli-ai-agent wants to merge 2 commits into
embroider-build:mainfrom
NullVoxPopuli-ai-agent:type-module-v2-addons
Open

Support type=module v2 addons#718
NullVoxPopuli-ai-agent wants to merge 2 commits into
embroider-build:mainfrom
NullVoxPopuli-ai-agent:type-module-v2-addons

Conversation

@NullVoxPopuli-ai-agent

Copy link
Copy Markdown

Part of the type=module quest issue: embroider-build/embroider#1773

Problem

When a v2 addon sets "type": "module" in its package.json, webpack applies strict ESM semantics to all of the addon's .js files, which breaks them in two ways that non-type=module addons never hit:

  1. Lost __esModule interop for externalized modules. Default-importing one of the CommonJS/AMD modules we externalize (like @ember/component/template-only, @glimmer/component when it's a v1 addon, or any other v1 addon) yields the module's exports object instead of its default export, because webpack (correctly, per spec) skips the __esModule interop for strict-ESM importers. At runtime this shows up as:

  2. Fully-specified resolution. Import specifiers inside the addon must carry file extensions, so directory imports (./lib./lib/index.js) fail, and so does the relative extensionless es-compat2 import that @embroider/macros emits when it compiles importSync() (importSync breaks the builds of apps consuming type=module packages embroider#1672):

    Module not found: Error: Can't resolve '../…/@embroider/macros/src/addon/es-compat2'
    BREAKING CHANGE: The request … failed to resolve only because it was resolved as fully specified
    

Solution

Add a webpack module rule that opts .js files owned by v2 addons back into type: 'javascript/auto' with resolve: { fullySpecified: false }. That is exactly the treatment every non-type=module v2 addon already gets (their .js files default to javascript/auto because their package.json has no type field), so type=module addons now behave identically to all other v2 addons. Packages that aren't ember addons are unaffected and keep spec-compliant strict ESM behavior.

Testing

  • New scenario v2-addon-type-module-test.ts with a type=module v2 addon covering: a template-only component, a @glimmer/component subclass, an internal directory import, and importSync() from @embroider/macros. All four fail without the fix and pass with it (release and releaseWithModules; beta/canary are skipped because rendering a component from a v2 addon is broken there even without type=module — same reason v2-addon-test.ts skips them).
  • Verified no regressions: v2-addon-test.ts and import-sync-test.ts (release scenarios incl. fastboot) all pass.
  • Also verified end-to-end against the quest repro repo (https://github.com/NullVoxPopuli/embroider-type-module-testing): with this fix linked in, the previously-failing app-auto-import-lib-one-template-only-component, app-auto-import-lib-glimmer-component, and app-auto-import-lib-import-sync apps all pass.

🤖 Generated with Claude Code

Ember addons need their .js files to be interpreted the same way whether
or not the addon's package.json says "type": "module". Without this,
webpack gives strict ESM treatment to .js files in v2 addons that say
type=module, which breaks them in two ways:

1. Default-importing one of the CommonJS/AMD modules that we externalize
   (like @ember/component/template-only, or any v1 addon) yields the
   module's exports object rather than its default export, because strict
   ESM importers don't get the __esModule interop. At runtime this shows
   up as errors like "_ember_component_template_only is not a function"
   (embroider-build/embroider#1774) or "Attempted to load a component,
   but there wasn't a component manager associated with the definition".

2. Import specifiers must be fully-specified, so directory imports and
   the relative extensionless es-compat2 import that @embroider/macros
   emits for importSync() fail to resolve
   (embroider-build/embroider#1672).

This adds a module rule that opts .js files owned by v2 addons back into
webpack's regular javascript/auto handling, which is exactly the
treatment every non-type=module v2 addon already gets.

Part of the type=module quest: embroider-build/embroider#1773

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@NullVoxPopuli-ai-agent

Copy link
Copy Markdown
Author

Note on the red CI: all release-* / releaseWithModules-* jobs are failing at app boot with

Uncaught Error: Could not find module `ember` imported from `ember-resolver/resolvers/classic/index`

including suites this PR doesn't touch (release-babel, release-csp, release-common-chunk, …). That's pre-existing ecosystem drift, not caused by this change: CI installs with use_lockfile: false, ember-source-latest is aliased to npm:ember-source@latest, and ember-source 7.x (which removed the ember barrel module) became latest after main's last CI run on May 6 (which was green). So every scenario that floats to ember-source latest now dies before any test code runs.

The new v2-addon-type-module scenarios pass locally against the committed lockfile (ember-source 6.12.0), and I verified no regressions in v2-addon-test and import-sync-test the same way.

Happy to take a stab at getting the test matrix onto ember-source 7 in a separate PR if that's welcome.

A v1 addon dependency is externalized to the runtime AMD loader, so
default-importing it from a strict-ESM file is one of the interop
shapes the javascript/auto rule exists to fix. Verified load-bearing:
with only fullySpecified:false (no type override), the component
renders and the importSync module fail with the strict-ESM interop and
runtime-require errors.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@NullVoxPopuli-ai-agent

Copy link
Copy Markdown
Author

Added explicit coverage for the "type=module v2 addon depends on a v1 addon" case: esm-v2-addon now declares inner-v1-addon as a dependency and both default-imports and named-imports from it. v1 addons are externalized to the runtime AMD loader, so the default import is exactly the __esModule-interop shape this PR fixes.

Also ran a sharper control locally: with only resolve: { fullySpecified: false } (no type: 'javascript/auto'), the build succeeds but the component renders fail with the interop errors and the importSync module dies at load with Could not find module ./side-effect.js imported from (require) — i.e. fullySpecified: false alone (the approach in #682/#707) is not sufficient; the module-type override is load-bearing.

(For completeness: @embroider/macros is also a v1 addon, but a compile-time one — its imports are compiled away by the babel plugin, and what's left at runtime are relative imports of real files inside the macros package. That's the fully-specified failure covered by the importSync test.)

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.

2 participants