From e44a64ab939185c04c2833b1e2972db9178f3ae7 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Wed, 11 Sep 2024 13:13:28 +0100 Subject: [PATCH] Mandatory staticEmberSource and no more AMD externals The older AMD externals system isn't really interoperable with true ES modules (and thus vite). We've kept it around this far, but the only way to actually use it is to manually register every single export that needs to be found in AMD, which is pretty clunkky. It's also not really needed. In a typical app, zero imports (actual import statements. The ember resolver is a separate topic.) are actually getting handled by the AMD loader. This PR drops the staticEmberSource option by forcing it to always be true, and drops the amdCompatibility option because it's no longer allowed. --- .../compat-adapters/ember-macro-helpers.ts | 12 -- .../src/compat-adapters/ember-source.ts | 71 +++++------ packages/compat/src/compat-app-builder.ts | 1 - packages/compat/src/options.ts | 12 -- packages/compat/tests/audit.test.ts | 1 - packages/core/src/module-resolver.ts | 94 +------------- packages/core/src/options.ts | 43 ------- packages/core/src/virtual-content.ts | 117 ------------------ packages/shared-internals/src/metadata.ts | 1 - tests/scenarios/compat-resolver-test.ts | 1 - tests/scenarios/core-resolver-test.ts | 1 - 11 files changed, 32 insertions(+), 322 deletions(-) delete mode 100644 packages/compat/src/compat-adapters/ember-macro-helpers.ts diff --git a/packages/compat/src/compat-adapters/ember-macro-helpers.ts b/packages/compat/src/compat-adapters/ember-macro-helpers.ts deleted file mode 100644 index 8921a9271..000000000 --- a/packages/compat/src/compat-adapters/ember-macro-helpers.ts +++ /dev/null @@ -1,12 +0,0 @@ -import V1Addon from '../v1-addon'; - -export default class extends V1Addon { - get packageMeta() { - let meta = super.packageMeta; - if (!meta.externals) { - meta.externals = []; - } - meta.externals.push('./-computed-store'); - return meta; - } -} diff --git a/packages/compat/src/compat-adapters/ember-source.ts b/packages/compat/src/compat-adapters/ember-source.ts index 0eb73cced..b919e0006 100644 --- a/packages/compat/src/compat-adapters/ember-source.ts +++ b/packages/compat/src/compat-adapters/ember-source.ts @@ -18,10 +18,6 @@ export default class extends V1Addon { return mergeTrees([super.v2Tree, buildFunnel(this.rootTree, { include: ['dist/ember-template-compiler.js'] })]); } - private get useStaticEmber(): boolean { - return this.app.options.staticEmberSource; - } - // versions of ember-source prior to // https://github.com/emberjs/ember.js/pull/20675 ship dist/packages and // dist/dependencies separately and the imports between them are package-name @@ -60,37 +56,30 @@ export default class extends V1Addon { get newPackageJSON() { let json = super.newPackageJSON; - if (this.useStaticEmber) { - for (let name of this.includedDependencies) { - // weirdly, many of the inlined dependency are still listed as real - // dependencies too. If we don't delete them here, they will take - // precedence over the inlined ones, because the embroider module-resolver - // tries to prioritize real deps. - delete json.dependencies?.[name]; - } + + for (let name of this.includedDependencies) { + // weirdly, many of the inlined dependency are still listed as real + // dependencies too. If we don't delete them here, they will take + // precedence over the inlined ones, because the embroider module-resolver + // tries to prioritize real deps. + delete json.dependencies?.[name]; } + return json; } customizes(treeName: string) { - if (this.useStaticEmber) { - // we are adding custom implementations of these - return treeName === 'treeForAddon' || treeName === 'treeForVendor' || super.customizes(treeName); - } else { - return super.customizes(treeName); - } + // we are adding custom implementations of these + return treeName === 'treeForAddon' || treeName === 'treeForVendor' || super.customizes(treeName); } - invokeOriginalTreeFor(name: string, opts: { neuterPreprocessors: boolean } = { neuterPreprocessors: false }) { - if (this.useStaticEmber) { - if (name === 'addon') { - return this.customAddonTree(); - } - if (name === 'vendor') { - return this.customVendorTree(); - } + invokeOriginalTreeFor(name: string) { + if (name === 'addon') { + return this.customAddonTree(); + } + if (name === 'vendor') { + return this.customVendorTree(); } - return super.invokeOriginalTreeFor(name, opts); } // Our addon tree is all of the "packages" we share. @embroider/compat already @@ -137,22 +126,22 @@ export default class extends V1Addon { get packageMeta() { let meta = super.packageMeta; - if (this.useStaticEmber) { - if (!meta['implicit-modules']) { - meta['implicit-modules'] = []; - } - meta['implicit-modules'].push('./ember/index.js'); - // before 5.6, Ember uses the AMD loader to decide if it's test-only parts - // are present, so we must ensure they're registered. After that it's - // enough to evaluate ember-testing, which @embroider/core is hard-coded - // to do in the backward-compatible tests bundle. - if (!satisfies(this.packageJSON.version, '>= 5.6.0-alpha.0', { includePrerelease: true })) { - if (!meta['implicit-test-modules']) { - meta['implicit-test-modules'] = []; - } - meta['implicit-test-modules'].push('./ember-testing/index.js'); + + if (!meta['implicit-modules']) { + meta['implicit-modules'] = []; + } + meta['implicit-modules'].push('./ember/index.js'); + // before 5.6, Ember uses the AMD loader to decide if it's test-only parts + // are present, so we must ensure they're registered. After that it's + // enough to evaluate ember-testing, which @embroider/core is hard-coded + // to do in the backward-compatible tests bundle. + if (!satisfies(this.packageJSON.version, '>= 5.6.0-alpha.0', { includePrerelease: true })) { + if (!meta['implicit-test-modules']) { + meta['implicit-test-modules'] = []; } + meta['implicit-test-modules'].push('./ember-testing/index.js'); } + return meta; } } diff --git a/packages/compat/src/compat-app-builder.ts b/packages/compat/src/compat-app-builder.ts index 4d143fed4..f22ab5f03 100644 --- a/packages/compat/src/compat-app-builder.ts +++ b/packages/compat/src/compat-app-builder.ts @@ -146,7 +146,6 @@ export class CompatAppBuilder { .reverse(), isLazy: engine.package.isLazyEngine(), })), - amdCompatibility: this.options.amdCompatibility, // this is the additional stufff that @embroider/compat adds on top to do // global template resolving diff --git a/packages/compat/src/options.ts b/packages/compat/src/options.ts index 3590b0599..a049225fa 100644 --- a/packages/compat/src/options.ts +++ b/packages/compat/src/options.ts @@ -43,16 +43,6 @@ export default interface Options extends CoreOptions { // apply. staticAddonTestSupportTrees?: boolean; - // when true, we will load ember-source as ES modules. This means unused parts - // of ember-source won't be included. But it also means that addons using old - // APIs to try to `require()` things from Ember -- particularly from within - // vendor.js -- cannot do that anymore. - // - // When false (the default) we load ember-source the traditional way, which is - // that a big ol' script gets smooshed into vendor.js, and none of ember's - // public module API actually exists as modules at build time. - staticEmberSource?: boolean; - // Allows you to override how specific addons will build. Like: // // import V1Addon from '@embroider/compat'; let compatAdapters = new Map(); @@ -112,7 +102,6 @@ export default interface Options extends CoreOptions { const defaults = Object.assign(coreWithDefaults(), { staticAddonTrees: false, staticAddonTestSupportTrees: false, - staticEmberSource: false, compatAdapters: new Map(), extraPublicTrees: [], workspaceDir: null, @@ -139,7 +128,6 @@ export const recommendedOptions: { [name: string]: Options } = Object.freeze({ staticHelpers: true, staticModifiers: true, staticComponents: true, - staticEmberSource: true, allowUnsafeDynamicComponents: false, }), }); diff --git a/packages/compat/tests/audit.test.ts b/packages/compat/tests/audit.test.ts index 7ab9ef8f2..b85299b67 100644 --- a/packages/compat/tests/audit.test.ts +++ b/packages/compat/tests/audit.test.ts @@ -40,7 +40,6 @@ describe('audit', function () { const resolvableExtensions = ['.js', '.hbs']; let resolverConfig: CompatResolverOptions = { - amdCompatibility: 'cjs', appRoot: app.baseDir, modulePrefix: 'audit-this-app', options: { diff --git a/packages/core/src/module-resolver.ts b/packages/core/src/module-resolver.ts index eed011d9f..51c77a72f 100644 --- a/packages/core/src/module-resolver.ts +++ b/packages/core/src/module-resolver.ts @@ -13,18 +13,10 @@ import assertNever from 'assert-never'; import { externalName } from '@embroider/reverse-exports'; import { exports as resolveExports } from 'resolve.exports'; -import { - virtualExternalESModule, - virtualExternalCJSModule, - virtualPairComponent, - fastbootSwitch, - decodeFastbootSwitch, - decodeImplicitModules, -} from './virtual-content'; +import { virtualPairComponent, fastbootSwitch, decodeFastbootSwitch, decodeImplicitModules } from './virtual-content'; import { Memoize } from 'typescript-memoize'; import { describeExports } from './describe-exports'; import { readFileSync } from 'fs'; -import type UserOptions from './options'; import { nodeResolve } from './node-resolve'; import { decodePublicRouteEntrypoint, encodeRouteEntrypoint } from './virtual-route-entrypoint'; @@ -94,7 +86,6 @@ export interface Options { modulePrefix: string; splitAtRoutes?: (RegExp | string)[]; podModulePrefix?: string; - amdCompatibility: Required; autoRun: boolean; staticAppPaths: string[]; emberVersion: string; @@ -191,10 +182,6 @@ export class Resolver { return logTransition('early exit', request); } - if (request.specifier === 'require') { - return this.external('early require', request, request.specifier); - } - request = this.handleFastbootSwitch(request); request = await this.handleGlobalsCompat(request); request = this.handleImplicitModules(request); @@ -1099,11 +1086,6 @@ export class Resolver { let packageName = getPackageName(specifier); if (!packageName) { - // This is a relative import. We don't automatically externalize those - // because it's rare, and by keeping them static we give better errors. But - // we do allow them to be explicitly externalized by the package author (or - // a compat adapter). In the metadata, they would be listed in - // package-relative form, so we need to convert this specifier to that. let absoluteSpecifier = resolve(dirname(fromFile), specifier); if (!absoluteSpecifier.startsWith(pkg.root)) { @@ -1114,12 +1096,6 @@ export class Resolver { return logTransition('beforeResolve: relative path escapes its package', request); } - let packageRelativeSpecifier = explicitRelative(pkg.root, absoluteSpecifier); - if (isExplicitlyExternal(packageRelativeSpecifier, pkg)) { - let publicSpecifier = absoluteSpecifier.replace(pkg.root, pkg.name); - return this.external('beforeResolve', request, publicSpecifier); - } - // if the requesting file is in an addon's app-js, the relative request // should really be understood as a request for a module in the containing // engine. @@ -1137,16 +1113,6 @@ export class Resolver { return request; } - // absolute package imports can also be explicitly external based on their - // full specifier name - if (isExplicitlyExternal(specifier, pkg)) { - return this.external('beforeResolve', request, specifier); - } - - if (emberVirtualPackages.has(packageName) && !pkg.hasDependency(packageName)) { - return this.external('beforeResolve emberVirtualPackages', request, specifier); - } - if (emberVirtualPeerDeps.has(packageName) && !pkg.hasDependency(packageName)) { // addons (whether auto-upgraded or not) may use the app's // emberVirtualPeerDeps, like "@glimmer/component" etc. @@ -1170,7 +1136,7 @@ export class Resolver { if (!dep.isEmberAddon()) { // classic ember addons can only import non-ember dependencies if they // have ember-auto-import. - return this.external('v1 package without auto-import', request, specifier); + return logTransition('v1 package without auto-import', request, request.notFound()); } } catch (err) { if (err.code !== 'MODULE_NOT_FOUND') { @@ -1213,43 +1179,6 @@ export class Resolver { } } - private external(label: string, request: R, specifier: string): R { - if (this.options.amdCompatibility === 'cjs') { - let filename = virtualExternalCJSModule(specifier); - return logTransition(label, request, request.virtualize(filename)); - } else if (this.options.amdCompatibility) { - let entry = this.options.amdCompatibility.es.find( - entry => entry[0] === specifier || entry[0] + '/index' === specifier - ); - if (!entry && request.specifier === 'require') { - entry = ['require', ['default', 'has']]; - } - if (!entry) { - throw new Error( - `[${request.debugType}] A module tried to resolve "${request.specifier}" and didn't find it (${label}). - - - Maybe a dependency declaration is missing? - - Remember that v1 addons can only import non-Ember-addon NPM dependencies if they include ember-auto-import in their dependencies. - - If this dependency is available in the AMD loader (because someone manually called "define()" for it), you can configure a shim like: - - amdCompatibility: { - es: [ - ["${request.specifier}", ["default", "yourNamedExportsGoHere"]], - ] - } - -` - ); - } - let filename = virtualExternalESModule(specifier, entry[1]); - return logTransition(label, request, request.virtualize(filename)); - } else { - throw new Error( - `Embroider's amdCompatibility option is disabled, but something tried to use it to access "${request.specifier}"` - ); - } - } - private async fallbackResolve(request: R): Promise { if (request.isVirtual) { throw new Error( @@ -1348,21 +1277,6 @@ export class Resolver { } } - if (pkg.needsLooseResolving() && (request.meta?.runtimeFallback ?? true)) { - // auto-upgraded packages can fall back to attempting to find dependencies at - // runtime. Native v2 packages can only get this behavior in the - // isExplicitlyExternal case above because they need to explicitly ask for - // externals. - return this.external('v1 catch-all fallback', request, request.specifier); - } else { - // native v2 packages don't automatically externalize *everything* the way - // auto-upgraded packages do, but they still externalize known and approved - // ember virtual packages (like @ember/component) - if (emberVirtualPackages.has(packageName)) { - return this.external('emberVirtualPackages', request, request.specifier); - } - } - // this is falling through with the original specifier which was // non-resolvable, which will presumably cause a static build error in stage3. return logTransition('fallbackResolve final exit', request); @@ -1534,10 +1448,6 @@ export class Resolver { } } -function isExplicitlyExternal(specifier: string, fromPkg: V2Package): boolean { - return Boolean(fromPkg.isV2Addon() && fromPkg.meta['externals'] && fromPkg.meta['externals'].includes(specifier)); -} - // we don't want to allow things that resolve only by accident that are likely // to break in other setups. For example: import your dependencies' // dependencies, or importing your own name from within a monorepo (which will diff --git a/packages/core/src/options.ts b/packages/core/src/options.ts index d957c6b26..0a2c605b6 100644 --- a/packages/core/src/options.ts +++ b/packages/core/src/options.ts @@ -71,49 +71,6 @@ export default interface Options { // useMethod optionally lets you pick which property within the module to use. // If not provided, we use the module.exports itself. pluginHints?: { resolve: string[]; useMethod?: string }[]; - - // Ember classically used a runtime AMD module loader. - // - // Embroider *can* locate the vast majority of modules statically, but when an - // addon is doing something highly dynamic (like injecting AMD `define()` - // statements directly into a