Merge pull request #2089 from embroider-build/finish-keep-app-dir

add required fixes to keep-app-dir PR

packages/compat/package.json

@@ -57,6 +57,7 @@
     "lodash": "^4.17.21",
     "pkg-up": "^3.1.0",
     "resolve": "^1.20.0",
+    "resolve.exports": "^2.0.2",
     "resolve-package-path": "^4.0.1",
     "semver": "^7.3.5",
     "symlink-or-copy": "^1.3.1",

packages/compat/src/dependency-rules.ts

@@ -1,7 +1,8 @@
 import type { Resolver } from '@embroider/core';
 import { getOrCreate } from '@embroider/core';
-import { resolve } from 'path';
+import { resolve as pathResolve, dirname } from 'path';
 import { satisfies } from 'semver';
+import { resolve as resolveExports } from 'resolve.exports';
 
 export interface PackageRules {
   // This whole set of rules will only apply when the given addon package
@@ -232,14 +233,20 @@ export function activePackageRules(
 
 export function appTreeRulesDir(root: string, resolver: Resolver) {
   let pkg = resolver.packageCache.ownerOfFile(root);
-  if (pkg?.isV2Addon()) {
-    // in general v2 addons can keep their app tree stuff in other places than
-    // "_app_" and we would need to check their package.json to see. But this code
-    // is only for applying packageRules to auto-upgraded v1 addons and apps, and
-    // those we always organize predictably.
-    return resolve(root, '_app_');
-  } else {
-    // auto-upgraded apps don't get an exist _app_ dir.
-    return root;
+  if (pkg) {
+    if (pkg.isV2Addon()) {
+      // in general v2 addons can keep their app tree stuff in other places than
+      // "_app_" and we would need to check their package.json to see. But this code
+      // is only for applying packageRules to auto-upgraded v1 addons and apps, and
+      // those we always organize predictably.
+      return pathResolve(root, '_app_');
+    } else {
+      // this is an app
+      let matched = resolveExports(pkg.packageJSON, './index.js');
+      if (matched) {
+        return dirname(pathResolve(root, matched[0]));
+      }
+    }
   }
+  return root;
 }

packages/compat/src/resolver-transform.ts

@@ -547,17 +547,17 @@ class TemplateResolver implements ASTPlugin {
 
       2. Have a mustache statement like: `{{something}}`, where `something` is:
 
-        a. Not a variable in scope (for example, there's no preceeding line 
+        a. Not a variable in scope (for example, there's no preceeding line
            like `<Parent as |something|>`)
         b. Does not start with `@` because that must be an argument from outside this template.
-        c. Does not contain a dot, like `some.thing` (because that case is classically 
+        c. Does not contain a dot, like `some.thing` (because that case is classically
            never a global component resolution that we would need to handle)
-        d. Does not start with `this` (this rule is mostly redundant with the previous rule, 
+        d. Does not start with `this` (this rule is mostly redundant with the previous rule,
            but even a standalone `this` is never a component invocation).
-        e. Does not have any arguments. If there are argument like `{{something a=b}}`, 
-           there is still ambiguity between helper vs component, but there is no longer 
+        e. Does not have any arguments. If there are argument like `{{something a=b}}`,
+           there is still ambiguity between helper vs component, but there is no longer
            the possibility that this was just rendering some data.
-        f. Does not take a block, like `{{#something}}{{/something}}` (because that is 
+        f. Does not take a block, like `{{#something}}{{/something}}` (because that is
            always a component, no ambiguity.)
 
     We can't tell if this problematic case is really:
@@ -571,7 +571,7 @@ class TemplateResolver implements ASTPlugin {
 
       2. A component invocation, which you could have written `<Something />`
          instead. Angle-bracket invocation has been available and easy-to-adopt
-         for a very long time. 
+         for a very long time.
 
       3. Property-this-fallback for `{{this.something}}`. Property-this-fallback
          is eliminated at Ember 4.0, so people have been heavily pushed to get

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/*',
+      },
     });
   });
 

packages/core/src/app-files.ts

@@ -9,7 +9,6 @@ export interface RouteFiles {
 }
 
 export class AppFiles {
-  readonly tests: ReadonlyArray<string>;
   readonly components: ReadonlyArray<string>;
   readonly helpers: ReadonlyArray<string>;
   readonly modifiers: ReadonlyArray<string>;
@@ -26,7 +25,6 @@ export class AppFiles {
     staticAppPathsPattern: RegExp | undefined,
     podModulePrefix?: string
   ) {
-    let tests: string[] = [];
     let components: string[] = [];
     let helpers: string[] = [];
     let modifiers: string[] = [];
@@ -78,7 +76,7 @@ export class AppFiles {
       }
 
       if (relativePath.startsWith('tests/')) {
-        tests.push(relativePath);
+        // skip tests because they are dealt with separately
         continue;
       }
 
@@ -134,7 +132,6 @@ export class AppFiles {
         otherAppFiles.push(relativePath);
       }
     }
-    this.tests = tests;
     this.components = components;
     this.helpers = helpers;
     this.modifiers = modifiers;

packages/core/src/module-resolver.ts

@@ -201,7 +201,6 @@ export class Resolver {
     request = this.handleVendorStyles(request);
     request = this.handleTestSupportStyles(request);
     request = this.handleEntrypoint(request);
-    request = this.handleTestEntrypoint(request);
     request = this.handleRouteEntrypoint(request);
     request = this.handleRenaming(request);
     request = this.handleVendor(request);
@@ -477,41 +476,6 @@ export class Resolver {
     );
   }
 
-  private handleTestEntrypoint<R extends ModuleRequest>(request: R): R {
-    if (isTerminal(request)) {
-      return request;
-    }
-
-    //TODO move the extra forwardslash handling out into the vite plugin
-    const candidates = [
-      '@embroider/core/test-entrypoint',
-      '/@embroider/core/test-entrypoint',
-      './@embroider/core/test-entrypoint',
-    ];
-
-    if (!candidates.some(c => request.specifier === c)) {
-      return request;
-    }
-
-    const pkg = this.packageCache.ownerOfFile(request.fromFile);
-
-    if (!pkg?.isV2Ember() || !pkg.isV2App()) {
-      throw new Error(
-        `bug: found test entrypoint import from somewhere other than the top-level app engine: ${request.fromFile}`
-      );
-    }
-
-    let matched = resolveExports(pkg.packageJSON, '-embroider-test-entrypoint.js', {
-      browser: true,
-      conditions: ['default', 'imports'],
-    });
-    return logTransition(
-      'test-entrypoint',
-      request,
-      request.virtualize(resolve(pkg.root, matched?.[0] ?? '-embroider-test-entrypoint.js'))
-    );
-  }
-
   private handleRouteEntrypoint<R extends ModuleRequest>(request: R): R {
     if (isTerminal(request)) {
       return request;
@@ -529,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<R extends ModuleRequest>(request: R): R {

packages/core/src/virtual-content.ts

@@ -8,7 +8,6 @@ import { decodeVirtualVendor, renderVendor } from './virtual-vendor';
 import { decodeVirtualVendorStyles, renderVendorStyles } from './virtual-vendor-styles';
 
 import { decodeEntrypoint, renderEntrypoint } from './virtual-entrypoint';
-import { decodeTestEntrypoint, renderTestEntrypoint } from './virtual-test-entrypoint';
 import { decodeRouteEntrypoint, renderRouteEntrypoint } from './virtual-route-entrypoint';
 
 const externalESPrefix = '/@embroider/ext-es/';
@@ -34,11 +33,6 @@ export function virtualContent(filename: string, resolver: Resolver): VirtualCon
     return renderEntrypoint(resolver, entrypoint);
   }
 
-  let testEntrypoint = decodeTestEntrypoint(filename);
-  if (testEntrypoint) {
-    return renderTestEntrypoint(resolver, testEntrypoint);
-  }
-
   let routeEntrypoint = decodeRouteEntrypoint(filename);
   if (routeEntrypoint) {
     return renderRouteEntrypoint(resolver, routeEntrypoint);

packages/core/src/virtual-route-entrypoint.ts

@@ -9,19 +9,19 @@ import { getAppFiles, getFastbootFiles, importPaths, splitRoute, staticAppPathsP
 
 const entrypointPattern = /(?<filename>.*)[\\/]-embroider-route-entrypoint.js:route=(?<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;
   }
   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),

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": {

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<PackageCache, 'ownerOfFile'>) {
+export 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');
+  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() {

packages/webpack/src/ember-webpack.ts

@@ -554,7 +554,7 @@ const Webpack: PackagerConstructor<Options> = class Webpack implements Packager
             // write all the stats output to the console
             this.consoleWrite(
               stats.toString({
-                color: Boolean(supportsColor.stdout),
+                colors: Boolean(supportsColor.stdout),
               })
             );
 
@@ -564,7 +564,7 @@ const Webpack: PackagerConstructor<Options> = class Webpack implements Packager
           if (stats.hasWarnings() || process.env.VANILLA_VERBOSE) {
             this.consoleWrite(
               stats.toString({
-                color: Boolean(supportsColor.stdout),
+                colors: Boolean(supportsColor.stdout),
               })
             );
           }