Merge pull request #1913 from embroider-build/virtual-compat-styles

virtualise engine styles with their vendor styles
embroider-build · May 16, 2024 · 51043f4 · 51043f4
2 parents d7336b2 + 0892a3a
commit 51043f4
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 89 deletions.
diff --git a/packages/compat/src/compat-app-builder.ts b/packages/compat/src/compat-app-builder.ts
@@ -27,7 +27,6 @@ import type { CompatResolverOptions } from './resolver-transform';
 import type { PackageRules } from './dependency-rules';
 import { activePackageRules } from './dependency-rules';
 import flatMap from 'lodash/flatMap';
-import sortBy from 'lodash/sortBy';
 import flatten from 'lodash/flatten';
 import partition from 'lodash/partition';
 import mergeWith from 'lodash/mergeWith';
@@ -39,7 +38,7 @@ import type { Options as EtcOptions } from 'babel-plugin-ember-template-compilat
 import type { Options as ResolverTransformOptions } from './resolver-transform';
 import type { Options as AdjustImportsOptions } from './babel-plugin-adjust-imports';
 import { PreparedEmberHTML } from '@embroider/core/src/ember-html';
-import type { InMemoryAsset, OnDiskAsset, ImplicitAssetPaths } from '@embroider/core/src/asset';
+import type { InMemoryAsset, OnDiskAsset } from '@embroider/core/src/asset';
 import { makePortable } from '@embroider/core/src/portable-babel-config';
 import type { RouteFiles } from '@embroider/core/src/app-files';
 import { AppFiles } from '@embroider/core/src/app-files';
@@ -288,64 +287,11 @@ export class CompatAppBuilder {
     return config;
   }
 
-  private scriptPriority(pkg: Package) {
-    switch (pkg.name) {
-      case 'loader.js':
-        return 0;
-      case 'ember-source':
-        return 10;
-      default:
-        return 1000;
-    }
-  }
-
   @Memoize()
   private get resolvableExtensionsPattern(): RegExp {
     return extensionsPattern(this.resolvableExtensions());
   }
 
-  private impliedAssets(type: keyof ImplicitAssetPaths, engine: AppFiles): (OnDiskAsset | InMemoryAsset)[] {
-    let result: (OnDiskAsset | InMemoryAsset)[] = this.impliedAddonAssets(type, engine).map(
-      (sourcePath: string): OnDiskAsset => {
-        let stats = statSync(sourcePath);
-        return {
-          kind: 'on-disk',
-          relativePath: explicitRelative(this.root, sourcePath),
-          sourcePath,
-          mtime: stats.mtimeMs,
-          size: stats.size,
-        };
-      }
-    );
-
-    return result;
-  }
-
-  private impliedAddonAssets(type: keyof ImplicitAssetPaths, { engine }: AppFiles): string[] {
-    let result: Array<string> = [];
-    for (let addon of sortBy(Array.from(engine.addons.keys()), this.scriptPriority.bind(this))) {
-      let implicitScripts = addon.meta[type];
-      if (implicitScripts) {
-        let styles = [];
-        let options = { basedir: addon.root };
-        for (let mod of implicitScripts) {
-          if (type === 'implicit-styles') {
-            // exclude engines because they will handle their own css importation
-            if (!addon.isLazyEngine()) {
-              styles.push(resolve.sync(mod, options));
-            }
-          } else {
-            result.push(resolve.sync(mod, options));
-          }
-        }
-        if (styles.length) {
-          result = [...styles, ...result];
-        }
-      }
-    }
-    return result;
-  }
-
   @Memoize()
   private async babelConfig(resolverConfig: CompatResolverOptions) {
     let babel = cloneDeep(this.compatApp.babelConfig());
@@ -1062,12 +1008,9 @@ export class CompatAppBuilder {
     // only import styles from engines with a parent (this excludeds the parent application) as their styles
     // will be inserted via a direct <link> tag.
     if (!appFiles.engine.isApp && appFiles.engine.package.isLazyEngine()) {
-      let implicitStyles = this.impliedAssets('implicit-styles', appFiles);
-      for (let style of implicitStyles) {
-        styles.push({
-          path: explicitRelative('assets/_engine_', style.relativePath),
-        });
-      }
+      styles.push({
+        path: '@embroider/core/vendor.css',
+      });
 
       let engineMeta = appFiles.engine.package.meta as AddonMeta;
       if (engineMeta && engineMeta['implicit-styles']) {

diff --git a/packages/core/src/asset.ts b/packages/core/src/asset.ts
@@ -1,13 +1,6 @@
 import type { JSDOM } from 'jsdom';
 import type { EmberHTML } from './ember-html';
 
-export interface ImplicitAssetPaths {
-  'implicit-scripts': string[];
-  'implicit-test-scripts': string[];
-  'implicit-styles': string[];
-  'implicit-test-styles': string[];
-}
-
 interface BaseAsset {
   // where this asset should be placed, relative to the app's root
   relativePath: string;

diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts
@@ -8,7 +8,7 @@ export {
 } from './packager';
 export { HTMLEntrypoint, BundleSummary } from './html-entrypoint';
 export { default as Stage } from './stage';
-export { Asset, EmberAsset, ImplicitAssetPaths } from './asset';
+export { Asset, EmberAsset } from './asset';
 export { default as Options, optionsWithDefaults } from './options';
 export { default as toBroccoliPlugin } from './to-broccoli-plugin';
 export { default as WaitForTrees, OutputPaths } from './wait-for-trees';

diff --git a/packages/core/src/module-resolver.ts b/packages/core/src/module-resolver.ts
@@ -511,9 +511,9 @@ export class Resolver {
     }
 
     let pkg = this.packageCache.ownerOfFile(request.fromFile);
-    if (pkg?.root !== this.options.engines[0].root) {
+    if (!pkg || !this.options.engines.some(e => e.root === pkg?.root)) {
       throw new Error(
-        `bug: found an import of ${request.specifier} in ${request.fromFile}, but this is not the top-level Ember app. The top-level Ember app is the only one that has support for @embroider/core/vendor.css. If you think something should be fixed in Embroider, please open an issue on https://github.com/embroider-build/embroider/issues.`
+        `bug: found an import of ${request.specifier} in ${request.fromFile}, but this is not the top-level Ember app or Engine. The top-level Ember app is the only one that has support for @embroider/core/vendor.css. If you think something should be fixed in Embroider, please open an issue on https://github.com/embroider-build/embroider/issues.`
       );
     }
 

diff --git a/packages/core/src/virtual-vendor-styles.ts b/packages/core/src/virtual-vendor-styles.ts
@@ -29,7 +29,7 @@ function getVendorStyles(owner: Package, resolver: Resolver): string {
         addon.canResolveFromFile,
       ])
     ),
-    isApp: true,
+    isApp: engineConfig.root === resolver.options.engines[0].root,
     modulePrefix: resolver.options.modulePrefix,
     appRelativePath: 'NOT_USED_DELETE_ME',
   };
@@ -38,10 +38,21 @@ function getVendorStyles(owner: Package, resolver: Resolver): string {
 }
 
 function generateVendorStyles(engine: Engine): string {
-  let result: string[] = impliedAddonVendorStyles(engine).map((sourcePath: string): string => {
-    let source = readFileSync(sourcePath);
-    return `${source}`;
-  });
+  let result: string[] = impliedAddonVendorStyles(engine).map(sourcePath => readFileSync(sourcePath, 'utf-8'));
+
+  // add the engines own styles but only if it is not the top-level app, that is provided by @embroider/synthesized-styles
+  if (!engine.isApp) {
+    let engineStyles = [];
+
+    engineStyles = getAddonImplicitStyles(engine.package as V2AddonPackage).map(sourcePath =>
+      readFileSync(sourcePath, 'utf-8')
+    );
+
+    // add engine's own implicit styles after all vendor styles
+    if (engineStyles.length) {
+      result = [...result, ...engineStyles];
+    }
+  }
 
   return result.join('') as string;
 }
@@ -58,20 +69,28 @@ function impliedAddonVendorStyles(engine: Engine): string[] {
         return 1000;
     }
   })) {
-    let implicitStyles = addon.meta['implicit-styles'];
-    if (implicitStyles) {
-      let styles = [];
-      let options = { basedir: addon.root };
-      for (let mod of implicitStyles) {
-        // exclude engines because they will handle their own css importation
-        if (!addon.isLazyEngine()) {
-          styles.push(resolve.sync(mod, options));
-        }
-      }
-      if (styles.length) {
-        result = [...styles, ...result];
-      }
+    // exclude lazy engines because they will handle their own css importation
+    if (addon.isLazyEngine()) {
+      continue;
+    }
+
+    let styles = getAddonImplicitStyles(addon);
+
+    if (styles.length) {
+      result = [...styles, ...result];
     }
   }
   return result;
 }
+
+function getAddonImplicitStyles(pkg: V2AddonPackage): string[] {
+  let implicitStyles = pkg.meta['implicit-styles'];
+  let styles = [];
+  if (implicitStyles) {
+    let options = { basedir: pkg.root };
+    for (let mod of implicitStyles) {
+      styles.push(resolve.sync(mod, options));
+    }
+  }
+  return styles;
+}