diff --git a/packages/macros/src/babel/state.ts b/packages/macros/src/babel/state.ts index 21785c8df1..6e3a39c8fa 100644 --- a/packages/macros/src/babel/state.ts +++ b/packages/macros/src/babel/state.ts @@ -69,8 +69,12 @@ const runtimeAddonPath = resolve(join(__dirname, '..', 'addon')); function pathToAddon(this: State, moduleName: string): string { if (!this.opts.owningPackageRoot) { - // running inside embroider, so make a relative path to the module - return explicitRelative(dirname(this.sourceFile), join(runtimeAddonPath, moduleName)); + // running inside embroider, so make a relative path to the module. The + // extension matters because this import may get emitted into a file + // whose package sets `"type": "module"`, where the bundler will apply + // strict ESM semantics and refuse to resolve import paths that aren't + // fully-specified. + return explicitRelative(dirname(this.sourceFile), join(runtimeAddonPath, `${moduleName}.js`)); } else { // running inside a classic build, so use a classic-compatible runtime // specifier. diff --git a/packages/macros/tests/babel/helpers.ts b/packages/macros/tests/babel/helpers.ts index ceebfa66b7..07f478230c 100644 --- a/packages/macros/tests/babel/helpers.ts +++ b/packages/macros/tests/babel/helpers.ts @@ -44,8 +44,7 @@ export function makeRunner(transform: Transform) { } return runDefault(code, { dependencies: { - [explicitRelative(dirname(optsWithDefaults.filename), runtimeFilename.replace(/\.[tj]s$/, ''))]: - cachedMacrosPackage, + [explicitRelative(dirname(optsWithDefaults.filename), runtimeFilename)]: cachedMacrosPackage, }, }); }; diff --git a/packages/macros/tests/babel/import-sync.test.ts b/packages/macros/tests/babel/import-sync.test.ts index 9d7536a4fe..bdbe015a55 100644 --- a/packages/macros/tests/babel/import-sync.test.ts +++ b/packages/macros/tests/babel/import-sync.test.ts @@ -11,7 +11,7 @@ describe('importSync', function () { import { importSync } from '@embroider/macros'; importSync('foo'); `); - expect(code).toMatch(/import esc from "\.\.\/\.\.\/src\/addon\/es-compat2"/); + expect(code).toMatch(/import esc from "\.\.\/\.\.\/src\/addon\/es-compat2\.js"/); expect(code).toMatch(/esc\(require\(['"]foo['"]\)\)/); expect(code).not.toMatch(/window/); }); diff --git a/packages/webpack/src/ember-webpack.ts b/packages/webpack/src/ember-webpack.ts index b6d64bc4ad..aaa8ae8b8a 100644 --- a/packages/webpack/src/ember-webpack.ts +++ b/packages/webpack/src/ember-webpack.ts @@ -11,7 +11,7 @@ import type { AppMeta, BundleSummary, Packager, PackagerConstructor, Variant, ResolverOptions } from '@embroider/core'; import { HTMLEntrypoint, getAppMeta, getPackagerCacheDir, getOrCreate } from '@embroider/core'; -import { locateEmbroiderWorkingDir, RewrittenPackageCache, tmpdir } from '@embroider/shared-internals'; +import { locateEmbroiderWorkingDir, PackageCache, RewrittenPackageCache, tmpdir } from '@embroider/shared-internals'; import type { Configuration, RuleSetUseItem, WebpackPluginInstance } from 'webpack'; import webpack from 'webpack'; import type { Stats } from 'fs-extra'; @@ -140,6 +140,11 @@ const Webpack: PackagerConstructor = class Webpack implements Packager warmUp(this.extraThreadLoaderOptions); } + private fileIsInV2Addon(filename: string): boolean { + let pkg = PackageCache.shared('embroider', this.appRoot).ownerOfFile(filename); + return Boolean(pkg?.isV2Addon()); + } + get bundleSummary(): BundleSummary { let bundleSummary = this._bundleSummary; if (bundleSummary === undefined) { @@ -224,6 +229,25 @@ const Webpack: PackagerConstructor = class Webpack implements Packager node: false, module: { rules: [ + { + // 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 rule, webpack would give the strict ESM + // treatment to .js files in v2 addons that say `"type": + // "module"`: import specifiers would need to be fully-specified + // (breaking, for example, the relative `es-compat2` import that + // @embroider/macros emits for importSync), and default-importing + // one of the CommonJS modules we externalize (like our + // `/@embroider/ext-cjs/` virtual modules) would yield the + // module's exports object rather than its default export. Opting + // these files back into "javascript/auto" keeps type=module v2 + // addons working the same as every other v2 addon. + test: (filename: string) => filename.endsWith('.js') && this.fileIsInV2Addon(filename), + type: 'javascript/auto', + resolve: { + fullySpecified: false, + }, + }, { test: /\.hbs$/, use: nonNullArray([ diff --git a/tests/scenarios/v2-addon-type-module-test.ts b/tests/scenarios/v2-addon-type-module-test.ts new file mode 100644 index 0000000000..2f3cfbf299 --- /dev/null +++ b/tests/scenarios/v2-addon-type-module-test.ts @@ -0,0 +1,214 @@ +import { appScenarios, baseAddon, baseV2Addon } from './scenarios'; +import type { PreparedApp } from 'scenario-tester'; +import QUnit from 'qunit'; +import merge from 'lodash/merge'; + +const { module: Qmodule, test } = QUnit; + +appScenarios + .map('v2-addon-type-module', project => { + // A v2 addon that sets `"type": "module"` in its package.json. All of + // its .js files get webpack's strict ESM treatment unless we intervene, + // which breaks (1) default-imports of the CommonJS modules we + // externalize (like the `/@embroider/ext-cjs/` virtual modules that + // stand in for `@ember/component/template-only` in safe mode), (2) + // imports that aren't fully-specified, like the relative `es-compat2` + // import that `@embroider/macros` emits for `importSync()`, and (3) + // `require()` itself, which strict ESM modules aren't allowed to use, so + // the `require()` calls that `importSync()` compiles to would be left + // for the runtime AMD loader instead of being handled by webpack. + let addon = baseV2Addon(); + addon.pkg.name = 'esm-v2-addon'; + addon.pkg.type = 'module'; + // when the package is type=module, the addon-main must be explicitly cjs + (addon.pkg as any)['ember-addon'].main = 'addon-main.cjs'; + addon.pkg.exports = { + '.': './index.js', + './*': './*', + }; + (addon.pkg as any)['ember-addon']['app-js'] = { + './components/esm-hello.js': './app/components/esm-hello.js', + './components/esm-counter.js': './app/components/esm-counter.js', + }; + + merge(addon.files, { + 'addon-main.cjs': ` + const { addonV1Shim } = require('@embroider/addon-shim'); + module.exports = addonV1Shim(__dirname); + `, + 'index.js': ` + import { two } from './lib'; + export function useDirectoryImport() { + return two(); + } + `, + lib: { + 'index.js': ` + export function two() { + return 'esm-directory-import-worked'; + } + `, + }, + 'side-effect.js': `window.__esm_v2_addon_side_effect = 'esm-side-effect-worked';`, + 'uses-import-sync.js': ` + import { importSync } from '@embroider/macros'; + importSync('./side-effect.js'); + `, + 'uses-v1-addon.js': ` + import innerV1Default, { innerV1Named } from 'inner-v1-addon'; + export function useV1AddonDefault() { + return innerV1Default(); + } + export function useV1AddonNamed() { + return innerV1Named(); + } + `, + app: { + components: { + 'esm-hello.js': `export { default } from 'esm-v2-addon/components/esm-hello';`, + 'esm-counter.js': `export { default } from 'esm-v2-addon/components/esm-counter';`, + }, + }, + components: { + 'esm-hello.js': ` + import { setComponentTemplate } from '@ember/component'; + import { precompileTemplate } from '@ember/template-compilation'; + import templateOnlyComponent from '@ember/component/template-only'; + export default setComponentTemplate( + precompileTemplate("
Hello from ESM
", { + strictMode: true, + }), + templateOnlyComponent() + ); + `, + 'esm-counter.js': ` + import Component from '@glimmer/component'; + import { setComponentTemplate } from '@ember/component'; + import { precompileTemplate } from '@ember/template-compilation'; + + class Counter extends Component { + get count() { + return 42; + } + } + + export default setComponentTemplate( + precompileTemplate("
{{this.count}}
", { + strictMode: true, + }), + Counter + ); + `, + }, + }); + + addon.linkDependency('@embroider/addon-shim', { baseDir: __dirname }); + addon.linkDependency('@embroider/macros', { baseDir: __dirname }); + + // a v1 addon consumed by the type=module v2 addon, to cover the request + // path where an import from a strict-ESM file lands in a rewritten + // package (see #1674) + let innerV1 = baseAddon(); + innerV1.pkg.name = 'inner-v1-addon'; + merge(innerV1.files, { + addon: { + 'index.js': ` + export default function innerV1Default() { + return 'inner-v1-default-worked'; + } + export function innerV1Named() { + return 'inner-v1-named-worked'; + } + `, + }, + }); + addon.addDependency(innerV1); + + project.addDevDependency(addon); + + merge(project.files, { + app: { + templates: { + 'index.hbs': ``, + }, + }, + tests: { + acceptance: { + 'esm-index-test.js': ` + import { module, test } from 'qunit'; + import { visit } from '@ember/test-helpers'; + import { setupApplicationTest } from 'ember-qunit'; + + module('Acceptance | index', function (hooks) { + setupApplicationTest(hooks); + + test('can render a template-only component from a type=module v2 addon', async function (assert) { + await visit('/'); + assert.equal(document.querySelector('[data-test-esm-hello]').textContent.trim(), 'Hello from ESM'); + }); + + test('can render a glimmer component from a type=module v2 addon', async function (assert) { + await visit('/'); + assert.equal(document.querySelector('[data-test-esm-counter]').textContent.trim(), '42'); + }); + }); + `, + }, + unit: { + 'esm-import-test.js': ` + import { module, test } from 'qunit'; + import { useDirectoryImport } from 'esm-v2-addon'; + import { useV1AddonDefault, useV1AddonNamed } from 'esm-v2-addon/uses-v1-addon'; + import 'esm-v2-addon/uses-import-sync'; + + module('Unit | import from type=module v2 addon', function () { + test('the addon can use a directory import internally', function (assert) { + assert.equal(useDirectoryImport(), 'esm-directory-import-worked'); + }); + + test('the addon can use importSync from @embroider/macros', function (assert) { + assert.equal(window.__esm_v2_addon_side_effect, 'esm-side-effect-worked'); + }); + + test('the addon can default-import from a v1 addon dependency', function (assert) { + assert.equal(useV1AddonDefault(), 'inner-v1-default-worked'); + }); + + test('the addon can named-import from a v1 addon dependency', function (assert) { + assert.equal(useV1AddonNamed(), 'inner-v1-named-worked'); + }); + }); + `, + }, + }, + }); + }) + .forEachScenario(scenario => { + Qmodule(scenario.name, function (hooks) { + let app: PreparedApp; + + hooks.before(async () => { + app = await scenario.prepare(); + }); + + test(`pnpm test: safe`, async function (assert) { + let result = await app.execute('pnpm test', { + env: { + EMBROIDER_TEST_SETUP_OPTIONS: 'safe', + EMBROIDER_TEST_SETUP_FORCE: 'embroider', + }, + }); + assert.equal(result.exitCode, 0, result.output); + }); + + test(`pnpm test: optimized`, async function (assert) { + let result = await app.execute('pnpm test', { + env: { + EMBROIDER_TEST_SETUP_OPTIONS: 'optimized', + EMBROIDER_TEST_SETUP_FORCE: 'embroider', + }, + }); + assert.equal(result.exitCode, 0, result.output); + }); + }); + });