Skip to content

@embroider/macros and type=module#1906

Draft
NullVoxPopuli wants to merge 13 commits intostablefrom
v2-addon-type-module-macros-resolve-build
Draft

@embroider/macros and type=module#1906
NullVoxPopuli wants to merge 13 commits intostablefrom
v2-addon-type-module-macros-resolve-build

Conversation

@NullVoxPopuli
Copy link
Copy Markdown
Collaborator

@NullVoxPopuli NullVoxPopuli commented May 7, 2024

Extracted from #1572 (which has more issues to fix (wrong css path, for example)

Two fixes here (specifically for type=module):

  • es-compat2 module was not resolvable (needed the js extensions)
  • importSync doesn't work with relative imports

@NullVoxPopuli NullVoxPopuli changed the base branch from main to stable May 7, 2024 14:08
@NullVoxPopuli NullVoxPopuli marked this pull request as draft May 7, 2024 14:08
@NullVoxPopuli NullVoxPopuli force-pushed the v2-addon-type-module-macros-resolve-build branch 2 times, most recently from 59b4fe7 to 09dbe55 Compare May 7, 2024 21:48
@NullVoxPopuli
Copy link
Copy Markdown
Collaborator Author

NullVoxPopuli commented May 7, 2024

According to:

require() of ES Module /tmp/tmp-31383035liZVSdehi7p/node_modules/v2-addon/addon-main.js 
  from <.pnpm>/[email protected]/node_modules/ember-cli/lib/models/package-info-cache/package-info.js 
  not supported.
addon-main.js is treated as an ES module file as it is a .js file 
  whose nearest parent package.json contains "type": "module" which declares all .js files 
  in that package scope as ES modules.
Instead rename addon-main.js to end in .cjs, change the requiring code to use dynamic 
  import() which is available in all CommonJS modules, or change "type": "module" to 
  "type": "commonjs" in /tmp/tmp-31383035liZVSdehi7p/node_modules/v2-addon/package.json 
  to treat all .js files as CommonJS (using .mjs for all ES modules instead).

It is required that addon-main.js be cjs.

I had reverted that change earlier to reduce diff, forgetting that .js is not allowed to be ambiguous with type=module (the ambiguity is relied on for current non-type=module v2 addons)

@NullVoxPopuli
Copy link
Copy Markdown
Collaborator Author

Current issue (and the one causing the two tests to hang):
image

Died on test #1: Could not find module `./side-effecting.js` imported from `(require)`

it seems that importSync isn't working on relative imports perhaps?

@NullVoxPopuli
Copy link
Copy Markdown
Collaborator Author

Each time I push, I'm going to be cancelling the workflow so as to not hog CI

@NullVoxPopuli NullVoxPopuli changed the title @embroider/macros es-compat module is resolvable from type=module addons @embroider/macros and type=module May 9, 2024
Addon-main is cjs, it should have cjs extension

Reproduction success

Make separate scenario

Test file must end with -test

Need to update the app's ember-cli-babel

Test is passing, now let's break it again...

Break successful
@NullVoxPopuli NullVoxPopuli force-pushed the v2-addon-type-module-macros-resolve-build branch from 1e33544 to 09d1ec5 Compare November 19, 2024 16:35
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.

1 participant