From 1d07369869597a4deb06b758b5d3fe6f6aa98209 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Fri, 9 Aug 2024 19:47:17 -0400 Subject: [PATCH 01/19] keep app dir --- packages/compat/src/compat-app.ts | 1 + packages/core/src/module-resolver.ts | 52 +++++++++++++------- packages/core/src/virtual-entrypoint.ts | 12 ++--- packages/core/src/virtual-test-entrypoint.ts | 12 ++--- tests/app-template/app/index.html | 4 +- tests/app-template/package.json | 3 +- 6 files changed, 50 insertions(+), 34 deletions(-) diff --git a/packages/compat/src/compat-app.ts b/packages/compat/src/compat-app.ts index 963df0d97..d231ceae5 100644 --- a/packages/compat/src/compat-app.ts +++ b/packages/compat/src/compat-app.ts @@ -618,6 +618,7 @@ export default class CompatApp { return this.preprocessJS( buildFunnel(this.legacyEmberAppInstance.trees.app, { exclude: ['styles/**', '*.html'], + destDir: 'app', }) ); } diff --git a/packages/core/src/module-resolver.ts b/packages/core/src/module-resolver.ts index 9c4a9846a..9d0c669ca 100644 --- a/packages/core/src/module-resolver.ts +++ b/packages/core/src/module-resolver.ts @@ -466,8 +466,15 @@ export class Resolver { } else { pkg = requestingPkg; } - - return logTransition('entrypoint', request, request.virtualize(resolve(pkg.root, '-embroider-entrypoint.js'))); + let matched = resolveExports(pkg.packageJSON, '-embroider-entrypoint.js', { + browser: true, + conditions: ['default', 'imports'], + }); + return logTransition( + 'entrypoint', + request, + request.virtualize(resolve(pkg.root, matched?.[0] ?? '-embroider-entrypoint.js')) + ); } private handleTestEntrypoint(request: R): R { @@ -494,10 +501,14 @@ export class Resolver { ); } + let matched = resolveExports(pkg.packageJSON, '-embroider-test-entrypoint.js', { + browser: true, + conditions: ['default', 'imports'], + }); return logTransition( 'test-entrypoint', request, - request.virtualize(resolve(pkg.root, '-embroider-test-entrypoint.js')) + request.virtualize(resolve(pkg.root, matched?.[0] ?? '-embroider-test-entrypoint.js')) ); } @@ -1333,11 +1344,15 @@ export class Resolver { if (withinEngine) { // it's a relative import inside an engine (which also means app), which // means we may need to satisfy the request via app tree merging. - let appJSMatch = await this.searchAppTree( - request, - withinEngine, - explicitRelative(pkg.root, resolve(dirname(request.fromFile), request.specifier)) - ); + + let logicalName = engineRelativeName(pkg, resolve(dirname(request.fromFile), request.specifier)); + if (!logicalName) { + return logTransition( + 'fallbackResolve: relative failure because this file is not externally accessible', + request + ); + } + let appJSMatch = await this.searchAppTree(request, withinEngine, logicalName); if (appJSMatch) { return logTransition('fallbackResolve: relative appJsMatch', request, appJSMatch); } else { @@ -1494,16 +1509,10 @@ export class Resolver { if (engineConfig) { // we're directly inside an engine, so we're potentially resolvable as a // global component - - // this kind of mapping is not true in general for all packages, but it - // *is* true for all classical engines (which includes apps) since they - // don't support package.json `exports`. As for a future v2 engine or app: - // this whole method is only relevant for implementing packageRules, which - // should only be for classic stuff. v2 packages should do the right - // things from the beginning and not need packageRules about themselves. - let inAppName = explicitRelative(engineConfig.root, filename); - - return this.tryReverseComponent(engineConfig.packageName, inAppName); + let inAppName = engineRelativeName(owningPackage, filename); + if (inAppName) { + return this.tryReverseComponent(engineConfig.packageName, inAppName); + } } let engineInfo = this.reverseSearchAppTree(owningPackage, filename); @@ -1555,3 +1564,10 @@ function reliablyResolvable(pkg: V2Package, packageName: string) { function appImportInAppTree(inPackage: Package, inLogicalPackage: Package, importedPackageName: string): boolean { return inPackage !== inLogicalPackage && importedPackageName === inLogicalPackage.name; } + +function engineRelativeName(pkg: Package, filename: string): string | undefined { + let outsideName = externalName(pkg.packageJSON, explicitRelative(pkg.root, filename)); + if (outsideName) { + return '.' + outsideName.slice(pkg.name.length); + } +} diff --git a/packages/core/src/virtual-entrypoint.ts b/packages/core/src/virtual-entrypoint.ts index 257e6b7a1..a74c5648a 100644 --- a/packages/core/src/virtual-entrypoint.ts +++ b/packages/core/src/virtual-entrypoint.ts @@ -12,7 +12,7 @@ import escapeRegExp from 'escape-string-regexp'; const entrypointPattern = /(?.*)[\\/]-embroider-entrypoint.js/; -export function decodeEntrypoint(filename: string): { fromFile: string } | undefined { +export function decodeEntrypoint(filename: string): { fromDir: string } | undefined { // Performance: avoid paying regex exec cost unless needed if (!filename.includes('-embroider-entrypoint')) { return; @@ -20,7 +20,7 @@ export function decodeEntrypoint(filename: string): { fromFile: string } | undef let m = entrypointPattern.exec(filename); if (m) { return { - fromFile: m.groups!.filename, + fromDir: m.groups!.filename, }; } } @@ -33,10 +33,10 @@ export function staticAppPathsPattern(staticAppPaths: string[] | undefined): Reg export function renderEntrypoint( resolver: Resolver, - { fromFile }: { fromFile: string } + { fromDir }: { fromDir: string } ): { src: string; watches: string[] } { // this is new - const owner = resolver.packageCache.ownerOfFile(fromFile); + const owner = resolver.packageCache.ownerOfFile(fromDir); let eagerModules: string[] = []; @@ -61,7 +61,7 @@ export function renderEntrypoint( modulePrefix: isApp ? resolver.options.modulePrefix : engine.packageName, appRelativePath: 'NOT_USED_DELETE_ME', }, - getAppFiles(owner.root), + getAppFiles(fromDir), hasFastboot ? getFastbootFiles(owner.root) : new Set(), extensionsPattern(resolver.options.resolvableExtensions), staticAppPathsPattern(resolver.options.staticAppPaths), @@ -154,8 +154,6 @@ export function renderEntrypoint( const entryTemplate = compile(` import { macroCondition, getGlobalConfig } from '@embroider/macros'; -import environment from './config/environment'; - {{#if styles}} if (macroCondition(!getGlobalConfig().fastboot?.isRunning)) { {{#each styles as |stylePath| ~}} diff --git a/packages/core/src/virtual-test-entrypoint.ts b/packages/core/src/virtual-test-entrypoint.ts index cb9168f81..12bfca844 100644 --- a/packages/core/src/virtual-test-entrypoint.ts +++ b/packages/core/src/virtual-test-entrypoint.ts @@ -7,7 +7,7 @@ import { getAppFiles, importPaths, staticAppPathsPattern } from './virtual-entry const entrypointPattern = /(?.*)[\\/]-embroider-test-entrypoint.js/; -export function decodeTestEntrypoint(filename: string): { fromFile: string } | undefined { +export function decodeTestEntrypoint(filename: string): { fromDir: string } | undefined { // Performance: avoid paying regex exec cost unless needed if (!filename.includes('-embroider-test-entrypoint.js')) { return; @@ -15,19 +15,19 @@ export function decodeTestEntrypoint(filename: string): { fromFile: string } | u let m = entrypointPattern.exec(filename); if (m) { return { - fromFile: m.groups!.filename, + fromDir: m.groups!.filename, }; } } export function renderTestEntrypoint( resolver: Resolver, - { fromFile }: { fromFile: string } + { fromDir }: { fromDir: string } ): { src: string; watches: string[] } { - const owner = resolver.packageCache.ownerOfFile(fromFile); + const owner = resolver.packageCache.ownerOfFile(fromDir); if (!owner) { - throw new Error(`Owner expected while loading test entrypoint from file: ${fromFile}`); + throw new Error(`Owner expected while loading test entrypoint from file: ${fromDir}`); } let engine = resolver.owningEngine(owner); @@ -45,7 +45,7 @@ export function renderTestEntrypoint( modulePrefix: resolver.options.modulePrefix, appRelativePath: 'NOT_USED_DELETE_ME', }, - getAppFiles(owner.root), + getAppFiles(fromDir), new Set(), // no fastboot files extensionsPattern(resolver.options.resolvableExtensions), staticAppPathsPattern(resolver.options.staticAppPaths), diff --git a/tests/app-template/app/index.html b/tests/app-template/app/index.html index c53dfc7da..60a4b93e5 100644 --- a/tests/app-template/app/index.html +++ b/tests/app-template/app/index.html @@ -18,8 +18,8 @@ diff --git a/tests/app-template/package.json b/tests/app-template/package.json index 978fc99dc..b179c5412 100644 --- a/tests/app-template/package.json +++ b/tests/app-template/package.json @@ -11,7 +11,8 @@ "test": "tests" }, "exports": { - "./*": "./*" + "./tests": "/tests/*", + "./*": "./app/*" }, "scripts": { "build": "vite build", From 2b939d77cb1ed267e383cc103458fefb77b9645c Mon Sep 17 00:00:00 2001 From: Chris Manson Date: Tue, 13 Aug 2024 15:54:12 +0100 Subject: [PATCH 02/19] fix app locations in all templates/fixtures --- .../addon-template/tests/dummy/app/index.html | 4 ++-- .../tests/dummy/app/index.html | 4 ++-- .../compat-addon-classic-features-test.ts | 18 +++++++++--------- tests/ts-app-template/app/index.html | 4 ++-- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/tests/addon-template/tests/dummy/app/index.html b/tests/addon-template/tests/dummy/app/index.html index c55e417ab..44af3dc9b 100644 --- a/tests/addon-template/tests/dummy/app/index.html +++ b/tests/addon-template/tests/dummy/app/index.html @@ -18,8 +18,8 @@ diff --git a/tests/fixtures/macro-sample-addon/tests/dummy/app/index.html b/tests/fixtures/macro-sample-addon/tests/dummy/app/index.html index 24d04a975..8dfc7c9bd 100644 --- a/tests/fixtures/macro-sample-addon/tests/dummy/app/index.html +++ b/tests/fixtures/macro-sample-addon/tests/dummy/app/index.html @@ -18,8 +18,8 @@ - + {{content-for "body-footer"}} diff --git a/tests/ts-app-template/app/index.html b/tests/ts-app-template/app/index.html index 8fcb46225..e25a32907 100644 --- a/tests/ts-app-template/app/index.html +++ b/tests/ts-app-template/app/index.html @@ -18,8 +18,8 @@ From 1627c7ecd57610f321ef6f7a55443291fab67e34 Mon Sep 17 00:00:00 2001 From: Chris Manson Date: Tue, 13 Aug 2024 15:54:32 +0100 Subject: [PATCH 03/19] fix relative imports from addon app-files --- packages/core/src/module-resolver.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/core/src/module-resolver.ts b/packages/core/src/module-resolver.ts index 9d0c669ca..c62df983d 100644 --- a/packages/core/src/module-resolver.ts +++ b/packages/core/src/module-resolver.ts @@ -1006,6 +1006,11 @@ export class Resolver { // ember-source might provide backburner via module renaming, but if you // have an explicit dependency on backburner you should still get that real // copy. + + // if (pkg.root === this.options.engines[0].root && request.specifier === `${pkg.name}/environment/config`) { + // return logTransition('legacy config location', request, request.alias(`${pkg.name}/app/environment/config`)); + // } + if (!pkg.hasDependency(packageName)) { for (let [candidate, replacement] of Object.entries(this.options.renameModules)) { if (candidate === request.specifier) { @@ -1148,14 +1153,15 @@ export class Resolver { // 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 + // engine. Because we're using a relative url we need to make sure that + // we're searching in the app folder let logicalLocation = this.reverseSearchAppTree(pkg, request.fromFile); if (logicalLocation) { return logTransition( 'beforeResolve: relative import in app-js', request, request - .alias('./' + posix.join(dirname(logicalLocation.inAppName), request.specifier)) + .alias('./app/' + posix.join(dirname(logicalLocation.inAppName), request.specifier)) // it's important that we're rehoming this to the root of the engine // (which we know really exists), and not to a subdir like // logicalLocation.inAppName (which might not physically exist), From a1b48f7327408f2f80337767d7149d6f8233ef87 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Tue, 20 Aug 2024 10:33:48 -0400 Subject: [PATCH 04/19] more progress on resolving through package.json exports --- packages/compat/src/compat-app-builder.ts | 7 +++ packages/core/src/module-resolver.ts | 59 ++++++++++------------- tests/app-template/package.json | 2 +- tests/scenarios/compat-dummy-app-test.ts | 3 +- 4 files changed, 35 insertions(+), 36 deletions(-) diff --git a/packages/compat/src/compat-app-builder.ts b/packages/compat/src/compat-app-builder.ts index 97b2fc975..a6f50740c 100644 --- a/packages/compat/src/compat-app-builder.ts +++ b/packages/compat/src/compat-app-builder.ts @@ -509,6 +509,13 @@ export class CompatAppBuilder { if ((this.origAppPackage.packageJSON['ember-addon']?.version ?? 0) < 2) { meta['auto-upgraded'] = true; + // our rewriting keeps app in app directory, etc. + pkgLayers.push({ + exports: { + './*': './app/*', + './tests/*': './tests/*', + }, + }); } pkgLayers.push({ 'ember-addon': meta }); diff --git a/packages/core/src/module-resolver.ts b/packages/core/src/module-resolver.ts index c62df983d..56f016147 100644 --- a/packages/core/src/module-resolver.ts +++ b/packages/core/src/module-resolver.ts @@ -1048,35 +1048,33 @@ export class Resolver { let owningEngine = this.owningEngine(pkg); let addonConfig = owningEngine.activeAddons.find(a => a.root === pkg.root); if (addonConfig) { + // auto-upgraded addons get special support for self-resolving here. return logTransition(`v1 addon self-import`, request, request.rehome(addonConfig.canResolveFromFile)); } else { - let selfImportPath = request.specifier === pkg.name ? './' : request.specifier.replace(pkg.name, '.'); + // auto-upgraded apps will necessarily have packageJSON.exports + // because we insert them, so for that support we can fall through to + // that support below. + } + } + + // v2 packages are supposed to use package.json `exports` to enable + // self-imports, but not all build tools actually follow the spec. This + // is a workaround for badly behaved packagers. + // + // Known upstream bugs this works around: + // - https://github.com/vitejs/vite/issues/9731 + if (pkg.packageJSON.exports) { + let found = resolveExports(pkg.packageJSON, request.specifier, { + browser: true, + conditions: ['default', 'imports'], + }); + if (found?.[0]) { return logTransition( - `v1 app self-import`, + `v2 self-import with package.json exports`, request, - request.alias(selfImportPath).rehome(resolve(pkg.root, 'package.json')) + request.alias(found?.[0]).rehome(resolve(pkg.root, 'package.json')) ); } - } else { - // v2 packages are supposed to use package.json `exports` to enable - // self-imports, but not all build tools actually follow the spec. This - // is a workaround for badly behaved packagers. - // - // Known upstream bugs this works around: - // - https://github.com/vitejs/vite/issues/9731 - if (pkg.packageJSON.exports) { - let found = resolveExports(pkg.packageJSON, request.specifier, { - browser: true, - conditions: ['default', 'imports'], - }); - if (found?.[0]) { - return logTransition( - `v2 self-import with package.json exports`, - request, - request.alias(found?.[0]).rehome(resolve(pkg.root, 'package.json')) - ); - } - } } } @@ -1153,22 +1151,15 @@ export class Resolver { // 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. Because we're using a relative url we need to make sure that - // we're searching in the app folder + // engine. let logicalLocation = this.reverseSearchAppTree(pkg, request.fromFile); if (logicalLocation) { return logTransition( 'beforeResolve: relative import in app-js', request, - request - .alias('./app/' + posix.join(dirname(logicalLocation.inAppName), request.specifier)) - // it's important that we're rehoming this to the root of the engine - // (which we know really exists), and not to a subdir like - // logicalLocation.inAppName (which might not physically exist), - // because some environments (including node's require.resolve) will - // refuse to do resolution from a notional path that doesn't - // physically exist. - .rehome(resolve(logicalLocation.owningEngine.root, 'package.json')) + request.alias( + posix.join(logicalLocation.owningEngine.packageName, dirname(logicalLocation.inAppName), request.specifier) + ) ); } diff --git a/tests/app-template/package.json b/tests/app-template/package.json index b179c5412..0d674cf66 100644 --- a/tests/app-template/package.json +++ b/tests/app-template/package.json @@ -11,7 +11,7 @@ "test": "tests" }, "exports": { - "./tests": "/tests/*", + "./tests/*": "./tests/*", "./*": "./app/*" }, "scripts": { diff --git a/tests/scenarios/compat-dummy-app-test.ts b/tests/scenarios/compat-dummy-app-test.ts index 1ffd69f6e..29c798a5a 100644 --- a/tests/scenarios/compat-dummy-app-test.ts +++ b/tests/scenarios/compat-dummy-app-test.ts @@ -76,7 +76,8 @@ dummyAppScenarios }); test('production build contains public assets from both addon and dummy app after a build', async function (assert) { - await app.execute(`pnpm vite build`); + let result = await app.execute(`pnpm vite build`); + assert.equal(result.exitCode, 0, result.output); let content = readFileSync(`${app.dir}/dist/robots.txt`).toString(); assert.strictEqual(content, 'go away bots'); content = readFileSync(`${app.dir}/dist/addon-template/from-addon.txt`).toString(); From 23b927d5a1151028651e6fa4dccf1bd3ca01c55d Mon Sep 17 00:00:00 2001 From: Chris Manson Date: Fri, 30 Aug 2024 21:09:48 +0100 Subject: [PATCH 05/19] update file paths in tests --- tests/scenarios/compat-exclude-dot-files-test.ts | 6 +++--- tests/scenarios/compat-preprocessors-test.ts | 7 +++++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/scenarios/compat-exclude-dot-files-test.ts b/tests/scenarios/compat-exclude-dot-files-test.ts index 0e6756640..270e4c508 100644 --- a/tests/scenarios/compat-exclude-dot-files-test.ts +++ b/tests/scenarios/compat-exclude-dot-files-test.ts @@ -79,9 +79,9 @@ appScenarios test('dot files are not included as app modules', function (assert) { // dot files should exist on disk - expectFile('./.foobar.js').exists(); - expectFile('./.barbaz.js').exists(); - expectFile('./bizbiz.js').exists(); + expectFile('./app/.foobar.js').exists(); + expectFile('./app/.barbaz.js').exists(); + expectFile('./app/bizbiz.js').exists(); // but not be picked up in the entrypoint expectAudit diff --git a/tests/scenarios/compat-preprocessors-test.ts b/tests/scenarios/compat-preprocessors-test.ts index f8bd78979..0a607db83 100644 --- a/tests/scenarios/compat-preprocessors-test.ts +++ b/tests/scenarios/compat-preprocessors-test.ts @@ -114,10 +114,13 @@ appScenarios }); test('app has correct path embedded in comment', () => { - const assertFile = expectFile('./components/from-the-app.js'); + const assertFile = expectFile('./app/components/from-the-app.js'); assertFile.exists(); // This is the expected output during an classic build. - assertFile.matches(/path@app-template\/components\/from-the-app\.js/, 'has a path comment in app components'); + assertFile.matches( + /path@app-template\/app\/components\/from-the-app\.js/, + 'has a path comment in app components' + ); }); test('addon has correct path embedded in comment', () => { From cb4c17134034fbc9d61af32022dbb02f140ab142 Mon Sep 17 00:00:00 2001 From: Chris Manson Date: Fri, 30 Aug 2024 21:39:26 +0100 Subject: [PATCH 06/19] fix isInComponents check --- packages/shared-internals/src/colocation.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared-internals/src/colocation.ts b/packages/shared-internals/src/colocation.ts index e22101555..cde38b2b5 100644 --- a/packages/shared-internals/src/colocation.ts +++ b/packages/shared-internals/src/colocation.ts @@ -42,7 +42,7 @@ function correspondingJSExists(id: string): boolean { function isInComponents(id: string, packageCache: Pick) { const pkg = packageCache.ownerOfFile(id); - return pkg?.isV2App() && id.slice(pkg?.root.length).split(sep).join('/').startsWith('/components'); + return pkg?.isV2App() && id.slice(pkg?.root.length).split(sep).join('/').startsWith('/app/components'); } export function templateOnlyComponentSource() { From 1c517ccfb5b64471398f64989000e9e6ccbf0674 Mon Sep 17 00:00:00 2001 From: Chris Manson Date: Fri, 30 Aug 2024 22:24:39 +0100 Subject: [PATCH 07/19] bring entrypoint updates to route entrypoints --- packages/core/src/module-resolver.ts | 11 ++++++++++- packages/core/src/virtual-route-entrypoint.ts | 14 +++++++------- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/packages/core/src/module-resolver.ts b/packages/core/src/module-resolver.ts index 2349408f8..bad3a9994 100644 --- a/packages/core/src/module-resolver.ts +++ b/packages/core/src/module-resolver.ts @@ -493,7 +493,16 @@ export class Resolver { throw new Error(`bug: found entrypoint import in non-ember package at ${request.fromFile}`); } - return logTransition('route entrypoint', request, request.virtualize(encodeRouteEntrypoint(pkg.root, routeName))); + let matched = resolveExports(pkg.packageJSON, '-embroider-route-entrypoint.js', { + browser: true, + conditions: ['default', 'imports'], + }); + + return logTransition( + 'route entrypoint', + request, + request.virtualize(encodeRouteEntrypoint(pkg.root, matched?.[0], routeName)) + ); } private handleImplicitTestScripts(request: R): R { diff --git a/packages/core/src/virtual-route-entrypoint.ts b/packages/core/src/virtual-route-entrypoint.ts index 82f589fe6..32a7af5a6 100644 --- a/packages/core/src/virtual-route-entrypoint.ts +++ b/packages/core/src/virtual-route-entrypoint.ts @@ -9,11 +9,11 @@ import { getAppFiles, getFastbootFiles, importPaths, splitRoute, staticAppPathsP const entrypointPattern = /(?.*)[\\/]-embroider-route-entrypoint.js:route=(?.*)/; -export function encodeRouteEntrypoint(packagePath: string, routeName: string): string { - return resolve(packagePath, `-embroider-route-entrypoint.js:route=${routeName}`); +export function encodeRouteEntrypoint(packagePath: string, matched: string | undefined, routeName: string): string { + return resolve(packagePath, `${matched}:route=${routeName}` ?? `-embroider-route-entrypoint.js:route=${routeName}`); } -export function decodeRouteEntrypoint(filename: string): { fromFile: string; route: string } | undefined { +export function decodeRouteEntrypoint(filename: string): { fromDir: string; route: string } | undefined { // Performance: avoid paying regex exec cost unless needed if (!filename.includes('-embroider-route-entrypoint')) { return; @@ -21,7 +21,7 @@ export function decodeRouteEntrypoint(filename: string): { fromFile: string; rou let m = entrypointPattern.exec(filename); if (m) { return { - fromFile: m.groups!.filename, + fromDir: m.groups!.filename, route: m.groups!.route, }; } @@ -42,9 +42,9 @@ export function decodePublicRouteEntrypoint(specifier: string): string | null { export function renderRouteEntrypoint( resolver: Resolver, - { fromFile, route }: { fromFile: string; route: string } + { fromDir, route }: { fromDir: string; route: string } ): { src: string; watches: string[] } { - const owner = resolver.packageCache.ownerOfFile(fromFile); + const owner = resolver.packageCache.ownerOfFile(fromDir); if (!owner) { throw new Error('Owner expected'); // ToDo: Really bad error, update message @@ -67,7 +67,7 @@ export function renderRouteEntrypoint( modulePrefix: isApp ? resolver.options.modulePrefix : engine.packageName, appRelativePath: 'NOT_USED_DELETE_ME', }, - getAppFiles(owner.root), + getAppFiles(fromDir), hasFastboot ? getFastbootFiles(owner.root) : new Set(), extensionsPattern(resolver.options.resolvableExtensions), staticAppPathsPattern(resolver.options.staticAppPaths), From 2cff65683cffb4aa0ac4150400e4c736095f584a Mon Sep 17 00:00:00 2001 From: Chris Manson Date: Fri, 30 Aug 2024 22:24:56 +0100 Subject: [PATCH 08/19] fix route-split-tests --- tests/scenarios/compat-route-split-test.ts | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/tests/scenarios/compat-route-split-test.ts b/tests/scenarios/compat-route-split-test.ts index 116b1f2b5..8cfefc913 100644 --- a/tests/scenarios/compat-route-split-test.ts +++ b/tests/scenarios/compat-route-split-test.ts @@ -68,7 +68,7 @@ function checkContents( .module('./index.html') .resolves(/\/index.html.*/) // in-html app-boot script .toModule() - .resolves(/\/app\.js.*/) + .resolves(/\/app\/app\.js.*/) .toModule() .resolves(/.*\/-embroider-entrypoint.js/); @@ -112,7 +112,9 @@ function inEntrypointFunction(expectAudit: ReturnType) { if (Array.isArray(text)) { text.forEach(t => { if (!contents.includes(t)) { - throw new Error(`${t} should be found in entrypoint`); + throw new Error(`${t} should be found in entrypoint: +--- +${contents}`); } }); } else if (text instanceof RegExp) { @@ -229,21 +231,21 @@ splitScenarios test('has split controllers in route entrypoint', function () { inEntrypoint( - ['controllers/people', 'controllers/people/show'], + ['app/controllers/people', 'app/controllers/people/show'], /@id\/embroider_virtual:.*-embroider-route-entrypoint.js:route=people/ ); }); test('has split route templates in route entrypoint', function () { inEntrypoint( - ['templates/people', 'templates/people/index', 'templates/people/show'], + ['app/templates/people', 'app/templates/people/index', 'app/templates/people/show'], /@id\/embroider_virtual:.*-embroider-route-entrypoint.js:route=people/ ); }); test('has split routes in route entrypoint', function () { inEntrypoint( - ['routes/people', 'routes/people/show'], + ['app/routes/people', 'app/routes/people/show'], /@id\/embroider_virtual:.*-embroider-route-entrypoint.js:route=people/ ); }); @@ -268,19 +270,19 @@ splitScenarios }); test('helper is consumed only from the template that uses it', function () { - expectAudit.module('./helpers/capitalize.js').hasConsumers(['./components/one-person.hbs']); + expectAudit.module('./app/helpers/capitalize.js').hasConsumers(['./app/components/one-person.hbs']); }); test('component is consumed only from the template that uses it', function () { - expectAudit.module('./components/one-person.js').hasConsumers(['./templates/people/show.hbs']); + expectAudit.module('./app/components/one-person.js').hasConsumers(['./app/templates/people/show.hbs']); }); test('modifier is consumed only from the template that uses it', function () { - expectAudit.module('./modifiers/auto-focus.js').hasConsumers(['./templates/people/edit.hbs']); + expectAudit.module('./app/modifiers/auto-focus.js').hasConsumers(['./app/templates/people/edit.hbs']); }); test('does not include unused component', function () { - expectAudit.module('./components/unused.hbs').doesNotExist(); + expectAudit.module('./app/components/unused.hbs').doesNotExist(); }); }); }); From 9c8979e298faeefafb72ca80a7fb0a90a3ab7271 Mon Sep 17 00:00:00 2001 From: Chris Manson Date: Fri, 30 Aug 2024 23:03:15 +0100 Subject: [PATCH 09/19] update file paths in tests --- tests/scenarios/compat-route-split-test.ts | 16 +++++----- tests/scenarios/compat-stage2-test.ts | 30 +++++++++---------- .../compat-template-colocation-test.ts | 8 ++--- 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/tests/scenarios/compat-route-split-test.ts b/tests/scenarios/compat-route-split-test.ts index 8cfefc913..4b05ebccb 100644 --- a/tests/scenarios/compat-route-split-test.ts +++ b/tests/scenarios/compat-route-split-test.ts @@ -446,19 +446,19 @@ splitScenarios }); test('helper is consumed only from the template that uses it', function () { - expectAudit.module('./helpers/capitalize.js').hasConsumers(['./components/one-person.hbs']); + expectAudit.module('./app/helpers/capitalize.js').hasConsumers(['./app/components/one-person.hbs']); }); test('component is consumed only from the template that uses it', function () { - expectAudit.module('./components/one-person.js').hasConsumers(['./pods/people/show/template.hbs']); + expectAudit.module('./app/components/one-person.js').hasConsumers(['./app/pods/people/show/template.hbs']); }); test('modifier is consumed only from the template that uses it', function () { - expectAudit.module('./modifiers/auto-focus.js').hasConsumers(['./pods/people/edit/template.hbs']); + expectAudit.module('./app/modifiers/auto-focus.js').hasConsumers(['./app/pods/people/edit/template.hbs']); }); test('does not include unused component', function () { - expectAudit.module('./components/unused.hbs').doesNotExist(); + expectAudit.module('./app/components/unused.hbs').doesNotExist(); }); }); }); @@ -622,19 +622,19 @@ splitScenarios }); test('helper is consumed only from the template that uses it', function () { - expectAudit.module('./helpers/capitalize.js').hasConsumers(['./components/one-person.hbs']); + expectAudit.module('./app/helpers/capitalize.js').hasConsumers(['./app/components/one-person.hbs']); }); test('component is consumed only from the template that uses it', function () { - expectAudit.module('./components/one-person.js').hasConsumers(['./routes/people/show/template.hbs']); + expectAudit.module('./app/components/one-person.js').hasConsumers(['./app/routes/people/show/template.hbs']); }); test('modifier is consumed only from the template that uses it', function () { - expectAudit.module('./modifiers/auto-focus.js').hasConsumers(['./routes/people/edit/template.hbs']); + expectAudit.module('./app/modifiers/auto-focus.js').hasConsumers(['./app/routes/people/edit/template.hbs']); }); test('does not include unused component', function () { - expectAudit.module('./components/unused.hbs').doesNotExist(); + expectAudit.module('./app/components/unused.hbs').doesNotExist(); }); }); }); diff --git a/tests/scenarios/compat-stage2-test.ts b/tests/scenarios/compat-stage2-test.ts index 53b6ceb0a..8663049b3 100644 --- a/tests/scenarios/compat-stage2-test.ts +++ b/tests/scenarios/compat-stage2-test.ts @@ -24,7 +24,7 @@ function resolveEntryPoint(expectAudit: ReturnType) { .module('./index.html') .resolves(/\/index.html.*/) // in-html app-boot script .toModule() - .resolves(/\/app\.js.*/) + .resolves(/\/app\/app\.js.*/) .toModule() .resolves(/.*\/-embroider-entrypoint.js/) .toModule(); @@ -563,7 +563,7 @@ stage2Scenarios }); test('index.hbs', function (assert) { - let expectModule = expectAudit.module('./templates/index.hbs'); + let expectModule = expectAudit.module('./app/templates/index.hbs'); // explicit dependency expectModule @@ -606,7 +606,7 @@ stage2Scenarios }); test('curly.hbs', function (assert) { - let expectModule = expectAudit.module('./templates/curly.hbs'); + let expectModule = expectAudit.module('./app/templates/curly.hbs'); expectModule .resolves(/my-addon\/_app_\/components\/hello-world/) .toModule() @@ -635,7 +635,7 @@ stage2Scenarios test('app/hello-world.js', function (assert) { expectAudit - .module('./templates/index.hbs') + .module('./app/templates/index.hbs') .resolves(/my-addon\/_app_\/components\/hello-world/) .toModule() .withContents(contents => { @@ -655,7 +655,7 @@ stage2Scenarios test('addon/hello-world.js', function (assert) { const expectModule = expectAudit - .module('./templates/index.hbs') + .module('./app/templates/index.hbs') .resolves(/my-addon\/_app_\/components\/hello-world/) .toModule() .resolves(/my-addon\/components\/hello-world\.js/) // remapped to precise copy of my-addon @@ -692,7 +692,7 @@ stage2Scenarios test('app/templates/components/direct-template-reexport.js', function (assert) { expectAudit - .module('./templates/index.hbs') + .module('./app/templates/index.hbs') .resolves(/my-addon\/_app_\/templates\/components\/direct-template-reexport\.js\/-embroider-pair-component/) .toModule() .resolves(/my-addon\/_app_\/templates\/components\/direct-template-reexport\.js/) @@ -707,7 +707,7 @@ stage2Scenarios test('uses-inline-template.js', function (assert) { expectAudit - .module('./components/uses-inline-template.js') + .module('./app/components/uses-inline-template.js') .resolves(/\/components\/first-choice.hbs\/-embroider-pair-component/) .toModule() .withContents(contents => { @@ -720,7 +720,7 @@ stage2Scenarios test('component with relative import of arbitrarily placed template', function () { expectAudit - .module(/\/app\.js.*/) + .module(/\/app\/app\.js.*/) .resolves(/.*\/-embroider-entrypoint\.js/) .toModule() .resolves(/.*\/-embroider-implicit-modules\.js/) @@ -734,7 +734,7 @@ stage2Scenarios test('app can import a deep addon', function () { expectAudit - .module('./use-deep-addon.js') + .module('./app/use-deep-addon.js') .resolves(/deep-addon\/index.js/) .toModule() .codeContains('export default function () {}'); @@ -771,27 +771,27 @@ stage2Scenarios }); test(`app's babel plugins ran`, async function () { - let assertFile = expectFile('custom-babel-needed.js').transform(build.transpile); + let assertFile = expectFile('app/custom-babel-needed.js').transform(build.transpile); assertFile.matches(/console\.log\(['"]embroider-sample-transforms-result['"]\)/); }); test('dynamic import is preserved', function () { - expectFile('./does-dynamic-import.js') + expectFile('./app/does-dynamic-import.js') .transform(build.transpile) .matches(/return import\(['"]some-library['"]\)/); }); test('hbs transform sees expected module name', function () { - let assertFile = expectFile('templates/components/module-name-check/index.hbs').transform(build.transpile); + let assertFile = expectFile('app/templates/components/module-name-check/index.hbs').transform(build.transpile); assertFile.matches( - '"my-app/templates/components/module-name-check/index.hbs"', + '"my-app/app/templates/components/module-name-check/index.hbs"', 'our sample transform injected the expected moduleName into the compiled template' ); }); test('non-static other paths are included in the entrypoint', function (assert) { resolveEntryPoint(expectAudit).withContents(contents => { - const result = /import \* as (\w+) from "\/non-static-dir\/another-library.js";/.exec(contents); + const result = /import \* as (\w+) from "\/app\/non-static-dir\/another-library.js";/.exec(contents); if (!result) { throw new Error('Could not find import for non-static-dir/another-library'); @@ -900,7 +900,7 @@ dummyAppScenarios }); test('dummy app sees that its being developed', function () { - let assertFile = expectFile('../../tmp/rewritten-app/components/inside-dummy-app.js').transform( + let assertFile = expectFile('../../tmp/rewritten-app/app/components/inside-dummy-app.js').transform( build.transpile ); assertFile.matches(/console\.log\(true\)/); diff --git a/tests/scenarios/compat-template-colocation-test.ts b/tests/scenarios/compat-template-colocation-test.ts index 02e4a0c39..7c55b2ca1 100644 --- a/tests/scenarios/compat-template-colocation-test.ts +++ b/tests/scenarios/compat-template-colocation-test.ts @@ -392,12 +392,12 @@ appScenarios .module('./index.html') .resolves(/\/index.html.*/) // in-html app-boot script .toModule() - .resolves(/\/app\.js.*/) + .resolves(/\/app\/app\.js.*/) .toModule() .resolves(/.*\/-embroider-entrypoint.js/) .toModule() .withContents(content => { - let result = /import \* as (\w+) from "\/components\/pod-component\/component\.js"/.exec(content); + let result = /import \* as (\w+) from "\/app\/components\/pod-component\/component\.js"/.exec(content); if (!result) { throw new Error('Could not find pod component'); @@ -409,7 +409,7 @@ appScenarios 'expected module is in the export list' ); - result = /import \* as (\w+) from "\/components\/pod-component\/template\.hbs.*"/.exec(content); + result = /import \* as (\w+) from "\/app\/components\/pod-component\/template\.hbs.*"/.exec(content); if (!result) { throw new Error('Could not find pod component template'); @@ -421,7 +421,7 @@ appScenarios 'expected module is in the export list' ); - result = /import \* as (\w+) from "\/components\/template-only\/template\.hbs.*"/.exec(content); + result = /import \* as (\w+) from "\/app\/components\/template-only\/template\.hbs.*"/.exec(content); if (!result) { throw new Error('Could not find template only component'); From d850898eb5c4ea3f58192e3746fcad0ea1180647 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Fri, 30 Aug 2024 18:35:21 -0400 Subject: [PATCH 10/19] follow package.json exports --- packages/shared-internals/package.json | 1 + packages/shared-internals/src/colocation.ts | 14 ++++++++++++-- pnpm-lock.yaml | 3 +++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/shared-internals/package.json b/packages/shared-internals/package.json index c6aa44a28..4da247271 100644 --- a/packages/shared-internals/package.json +++ b/packages/shared-internals/package.json @@ -37,6 +37,7 @@ "fs-extra": "^9.1.0", "lodash": "^4.17.21", "minimatch": "^3.0.4", + "resolve.exports": "^2.0.2", "semver": "^7.3.5" }, "devDependencies": { diff --git a/packages/shared-internals/src/colocation.ts b/packages/shared-internals/src/colocation.ts index cde38b2b5..8a732ad91 100644 --- a/packages/shared-internals/src/colocation.ts +++ b/packages/shared-internals/src/colocation.ts @@ -2,6 +2,7 @@ import { existsSync } from 'fs-extra'; import { cleanUrl } from './paths'; import type PackageCache from './package-cache'; import { sep } from 'path'; +import { resolve as resolveExports } from 'resolve.exports'; export function syntheticJStoHBS(source: string): string | null { // explicit js is the only case we care about here. Synthetic template JS is @@ -40,9 +41,18 @@ function correspondingJSExists(id: string): boolean { return ['js', 'ts'].some(ext => existsSync(id.slice(0, -3) + ext)); } -function isInComponents(id: string, packageCache: Pick) { +export function isInComponents(id: string, packageCache: Pick) { const pkg = packageCache.ownerOfFile(id); - return pkg?.isV2App() && id.slice(pkg?.root.length).split(sep).join('/').startsWith('/app/components'); + if (!pkg?.isV2App()) { + return false; + } + + let tryResolve = resolveExports(pkg.packageJSON, './components', { + browser: true, + conditions: ['default', 'imports'], + }); + let componentsDir = tryResolve?.[0] ?? './components'; + return ('.' + id.slice(pkg?.root.length).split(sep).join('/')).startsWith(componentsDir); } export function templateOnlyComponentSource() { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 14e9e146d..063565eae 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -710,6 +710,9 @@ importers: resolve-package-path: specifier: ^4.0.1 version: 4.0.3 + resolve.exports: + specifier: ^2.0.2 + version: 2.0.2 semver: specifier: ^7.3.5 version: 7.6.3 From 0dd42fedc7fe2f639482062b4575a76592928640 Mon Sep 17 00:00:00 2001 From: Chris Manson Date: Sat, 31 Aug 2024 16:43:21 +0100 Subject: [PATCH 11/19] update file paths in tests --- tests/scenarios/compat-template-colocation-test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/scenarios/compat-template-colocation-test.ts b/tests/scenarios/compat-template-colocation-test.ts index 7c55b2ca1..b1862cfcc 100644 --- a/tests/scenarios/compat-template-colocation-test.ts +++ b/tests/scenarios/compat-template-colocation-test.ts @@ -158,7 +158,7 @@ module('Integration | Component | addon-component-one', function (hooks) { expectAudit, contents => { assert.ok( - /import TEMPLATE from ['"]\/components\/has-colocated-template.hbs.*['"];/.test(contents), + /import TEMPLATE from ['"]\/app\/components\/has-colocated-template.hbs.*['"];/.test(contents), 'imported template' ); assert.ok(/import \{ setComponentTemplate \}/.test(contents), 'found setComponentTemplate'); @@ -176,7 +176,7 @@ module('Integration | Component | addon-component-one', function (hooks) { expectAudit, contents => { assert.ok( - /import TEMPLATE from ['"]\/components\/template-only-component.hbs.*['"];/.test(contents), + /import TEMPLATE from ['"]\/app\/components\/template-only-component.hbs.*['"];/.test(contents), 'imported template' ); assert.ok(/import \{ setComponentTemplate \}/.test(contents), 'found setComponentTemplate'); @@ -193,7 +193,7 @@ module('Integration | Component | addon-component-one', function (hooks) { test(`app's colocated components are implicitly included correctly`, function (assert) { checkContents(expectAudit, contents => { - const result = /import \* as (\w+) from "\/components\/has-colocated-template.js.*";/.exec(contents); + const result = /import \* as (\w+) from "\/app\/components\/has-colocated-template.js.*";/.exec(contents); if (!result) { console.log(contents); From 4461cb1ab1e9d8f4f8c4da318f40efdcf4c3f12c Mon Sep 17 00:00:00 2001 From: Chris Manson Date: Sat, 31 Aug 2024 17:21:15 +0100 Subject: [PATCH 12/19] fix resolver test - app resolving requires exports now --- tests/scenarios/core-resolver-test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/scenarios/core-resolver-test.ts b/tests/scenarios/core-resolver-test.ts index 764cc19b3..583dbd575 100644 --- a/tests/scenarios/core-resolver-test.ts +++ b/tests/scenarios/core-resolver-test.ts @@ -30,6 +30,10 @@ Scenarios.fromProject(() => new Project()) name: 'my-app', keywords: ['ember-addon'], 'ember-addon': appMeta as any, + exports: { + './*': './*', + './tests/*': './tests/*', + }, }; app.mergeFiles({ 'index.html': '', From e28730f12a52fe379f3cfafb2c3cb4651ac0b721 Mon Sep 17 00:00:00 2001 From: Chris Manson Date: Sun, 1 Sep 2024 10:46:05 +0100 Subject: [PATCH 13/19] add exports to audit test --- packages/compat/tests/audit.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/compat/tests/audit.test.ts b/packages/compat/tests/audit.test.ts index d6b40712e..4634d5cad 100644 --- a/packages/compat/tests/audit.test.ts +++ b/packages/compat/tests/audit.test.ts @@ -104,6 +104,10 @@ describe('audit', function () { merge(app.pkg, { 'ember-addon': appMeta, keywords: ['ember-addon'], + exports: { + './*': './*', + './tests/*': './tests/*', + }, }); }); From f86c4a9e437e687a533d937caeb1b4c25944d9d4 Mon Sep 17 00:00:00 2001 From: Chris Manson Date: Sun, 1 Sep 2024 10:46:54 +0100 Subject: [PATCH 14/19] fix typescript app import --- tests/ts-app-template/app/index.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ts-app-template/app/index.html b/tests/ts-app-template/app/index.html index e25a32907..3c632d832 100644 --- a/tests/ts-app-template/app/index.html +++ b/tests/ts-app-template/app/index.html @@ -18,7 +18,7 @@