Merge pull request embroider-build#2029 from embroider-build/vite-opt…

…imize-test Reform extension searching and dep optimization
PavinStrickland · Jul 16, 2024 · 0b5db9f · 0b5db9f
2 parents 203b5bc + 8170bd8
commit 0b5db9f
Show file tree

Hide file tree

Showing 23 changed files with 647 additions and 774 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -8,8 +8,7 @@ on:
   push:
     branches:
       - main
-  pull_request:
-    branches: [main]
+  pull_request: {}
 
 concurrency:
   group: ci-${{ github.head_ref || github.ref }}

diff --git a/packages/compat/src/compat-adapters/ember-resolver.ts b/packages/compat/src/compat-adapters/ember-resolver.ts
@@ -0,0 +1,15 @@
+import V1Addon from '../v1-addon';
+
+export default class extends V1Addon {
+  get packageMeta() {
+    let meta = super.packageMeta;
+    if (meta['implicit-modules']) {
+      // ember-resolver has a vestigial empty file here that existed due to
+      // babel-plugin-debug-macros behavior. But ember-resolver no longer uses
+      // babel-plugin-debug-macros. And the empty file makes vite's CJS interop
+      // get confused and produce a runtime crash.
+      meta['implicit-modules'] = meta['implicit-modules'].filter(m => m !== './features');
+    }
+    return meta;
+  }
+}
diff --git a/packages/core/src/module-resolver.ts b/packages/core/src/module-resolver.ts
@@ -1007,15 +1007,21 @@ export class Resolver {
         // supported fancy package.json exports features so this direct mapping
         // to the root is always right.
 
-        // "my-package/foo" -> "./foo"
-        // "my-package" -> "./" (this can't be just "." because node's require.resolve doesn't reliable support that)
-        let selfImportPath = request.specifier === pkg.name ? './' : request.specifier.replace(pkg.name, '.');
+        // "my-app/foo" -> "./foo" from app's package.json
+        // "my-addon/foo" -> "my-addon/foo" from a package that's guaranteed to be able to resolve my-addon
 
-        return logTransition(
-          `v1 self-import`,
-          request,
-          request.alias(selfImportPath).rehome(resolve(pkg.root, 'package.json'))
-        );
+        let owningEngine = this.owningEngine(pkg);
+        let addonConfig = owningEngine.activeAddons.find(a => a.root === pkg.root);
+        if (addonConfig) {
+          return logTransition(`v1 addon self-import`, request, request.rehome(addonConfig.canResolveFromFile));
+        } else {
+          let selfImportPath = request.specifier === pkg.name ? './' : request.specifier.replace(pkg.name, '.');
+          return logTransition(
+            `v1 app self-import`,
+            request,
+            request.alias(selfImportPath).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
@@ -1223,7 +1229,7 @@ export class Resolver {
       }
       if (!entry) {
         throw new Error(
-          `A module tried to resolve "${request.specifier}" and didn't find it (${label}).
+          `[${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.

diff --git a/packages/shared-internals/src/colocation.ts b/packages/shared-internals/src/colocation.ts
@@ -0,0 +1,50 @@
+import { existsSync } from 'fs-extra';
+import { cleanUrl } from './paths';
+import type PackageCache from './package-cache';
+import { sep } from 'path';
+
+export function syntheticJStoHBS(source: string): string | null {
+  // explicit js is the only case we care about here. Synthetic template JS is
+  // only ever JS (never TS or anything else). And extensionless imports are
+  // handled by the default resolving system doing extension search.
+  if (cleanUrl(source, true).endsWith('.js')) {
+    return source.replace(/.js(\?.*)?/, '.hbs$1');
+  }
+
+  return null;
+}
+
+export function needsSyntheticComponentJS(
+  requestedSpecifier: string,
+  foundFile: string,
+  packageCache: Pick<PackageCache, 'ownerOfFile'>
+): string | null {
+  requestedSpecifier = cleanUrl(requestedSpecifier, true);
+  foundFile = cleanUrl(foundFile);
+  if (
+    discoveredImplicitHBS(requestedSpecifier, foundFile) &&
+    !foundFile.split(sep).join('/').endsWith('/template.hbs') &&
+    !correspondingJSExists(foundFile) &&
+    isInComponents(foundFile, packageCache)
+  ) {
+    return foundFile.slice(0, -3) + 'js';
+  }
+  return null;
+}
+
+function discoveredImplicitHBS(source: string, id: string): boolean {
+  return !source.endsWith('.hbs') && id.endsWith('.hbs');
+}
+
+function correspondingJSExists(id: string): boolean {
+  return ['js', 'ts'].some(ext => existsSync(id.slice(0, -3) + ext));
+}
+
+function isInComponents(id: string, packageCache: Pick<PackageCache, 'ownerOfFile'>) {
+  const pkg = packageCache.ownerOfFile(id);
+  return pkg?.isV2App() && id.slice(pkg?.root.length).split(sep).join('/').startsWith('/components');
+}
+
+export function templateOnlyComponentSource() {
+  return `import templateOnly from '@ember/component/template-only';\nexport default templateOnly();\n`;
+}
diff --git a/packages/shared-internals/src/index.ts b/packages/shared-internals/src/index.ts
@@ -32,3 +32,4 @@ export {
 export { locateEmbroiderWorkingDir } from './working-dir';
 
 export * from './dep-validation';
+export * from './colocation';
diff --git a/packages/shared-internals/src/paths.ts b/packages/shared-internals/src/paths.ts
@@ -12,7 +12,7 @@ export function explicitRelative(fromDir: string, toFile: string) {
   if (!isAbsolute(result) && !result.startsWith('.')) {
     result = './' + result;
   }
-  if (isAbsolute(toFile) && result.endsWith(toFile)) {
+  if (isAbsolute(toFile) && result.split(sep).join('/').endsWith(toFile)) {
     // this prevents silly "relative" paths like
     // "../../../../../Users/you/projects/your/stuff" when we could have just
     // said "/Users/you/projects/your/stuff". The silly path isn't incorrect,

diff --git a/packages/vite/src/esbuild-request.ts b/packages/vite/src/esbuild-request.ts
@@ -16,7 +16,7 @@ export class EsBuildModuleRequest implements ModuleRequest {
       return;
     }
 
-    if (source && importer && source[0] !== '\0') {
+    if (source && importer && source[0] !== '\0' && !source.startsWith('virtual-module:')) {
       let fromFile = cleanUrl(importer);
       return new EsBuildModuleRequest(
         context,
@@ -130,7 +130,7 @@ export class EsBuildModuleRequest implements ModuleRequest {
       return {
         type: 'found',
         filename: request.specifier,
-        result: { path: request.specifier, namespace: 'embroider' },
+        result: { path: request.specifier, namespace: 'embroider-virtual' },
         isVirtual: this.isVirtual,
       };
     }
@@ -144,6 +144,8 @@ export class EsBuildModuleRequest implements ModuleRequest {
       };
     }
 
+    requestStatus(request.specifier);
+
     let result = await this.context.resolve(request.specifier, {
       importer: request.fromFile,
       resolveDir: dirname(request.fromFile),
@@ -155,7 +157,10 @@ export class EsBuildModuleRequest implements ModuleRequest {
         },
       },
     });
-    if (result.errors.length > 0) {
+
+    let status = readStatus(request.specifier);
+
+    if (result.errors.length > 0 || status === 'not_found') {
       return { type: 'not_found', err: result };
     } else if (result.external) {
       return { type: 'ignored', result };
@@ -164,3 +169,41 @@ export class EsBuildModuleRequest implements ModuleRequest {
     }
   }
 }
+
+/*
+  This is an unfortunate necessity. During depscan, vite deliberately hides
+  information from esbuild. Specifically, it treats "not found" and "this is an
+  external dependency" as both "external: true". But we really care about the
+  difference, since we have fallback behaviors for the "not found" case. Using
+  this global state, our rollup resolver plugin can observe what vite is
+  actually doing and communicate that knowledge outward to our esbuild resolver
+  plugin.
+ */
+function sharedGlobalState() {
+  let channel = (globalThis as any).__embroider_vite_resolver_channel__ as
+    | undefined
+    | Map<string, 'pending' | 'found' | 'not_found'>;
+  if (!channel) {
+    channel = new Map();
+    (globalThis as any).__embroider_vite_resolver_channel__ = channel;
+  }
+  return channel;
+}
+
+function requestStatus(id: string): void {
+  sharedGlobalState().set(id, 'pending');
+}
+
+export function writeStatus(id: string, status: 'found' | 'not_found'): void {
+  let channel = sharedGlobalState();
+  if (channel.get(id) === 'pending') {
+    channel.set(id, status);
+  }
+}
+
+function readStatus(id: string): 'pending' | 'not_found' | 'found' {
+  let channel = sharedGlobalState();
+  let result = channel.get(id) ?? 'pending';
+  channel.delete(id);
+  return result;
+}
Original file line number	Diff line number	Diff line change
Expand Up		@@ -32,3 +32,4 @@ export {
		export { locateEmbroiderWorkingDir } from './working-dir';

		export * from './dep-validation';
		export * from './colocation';