Merge pull request #1859 from embroider-build/public-assets-cleanup-s…

…tage-2

Public assets handling/ clean up stage 2

packages/compat/src/compat-app-builder.ts

@@ -20,7 +20,6 @@ import {
   Resolver,
   locateEmbroiderWorkingDir,
 } from '@embroider/core';
-import walkSync from 'walk-sync';
 import { resolve as resolvePath, posix } from 'path';
 import type { JSDOM } from 'jsdom';
 import type Options from './options';
@@ -90,23 +89,6 @@ export class CompatAppBuilder {
   private extractAssets(treePaths: OutputPaths<TreeNames>): Asset[] {
     let assets: Asset[] = [];
 
-    // Everything in our traditional public tree is an on-disk asset
-    if (treePaths.publicTree) {
-      walkSync
-        .entries(treePaths.publicTree, {
-          directories: false,
-        })
-        .forEach(entry => {
-          assets.push({
-            kind: 'on-disk',
-            relativePath: entry.relativePath,
-            sourcePath: entry.fullPath,
-            mtime: entry.mtime as unknown as number, // https://github.com/joliss/node-walk-sync/pull/38
-            size: entry.size,
-          });
-        });
-    }
-
     // ember-cli traditionally outputs a dummy testem.js file to prevent
     // spurious errors when running tests under "ember s".
     if (this.compatApp.shouldBuildTests) {
@@ -822,19 +804,19 @@ export class CompatAppBuilder {
   private gatherAssets(inputPaths: OutputPaths<TreeNames>): Asset[] {
     // first gather all the assets out of addons
     let assets: Asset[] = [];
-    for (let pkg of this.allActiveAddons) {
-      if (pkg.meta['public-assets']) {
-        for (let [filename, appRelativeURL] of Object.entries(pkg.meta['public-assets'] || {})) {
-          let sourcePath = resolvePath(pkg.root, filename);
-          let stats = statSync(sourcePath);
-          assets.push({
-            kind: 'on-disk',
-            sourcePath,
-            relativePath: appRelativeURL,
-            mtime: stats.mtimeMs,
-            size: stats.size,
-          });
-        }
+
+    let pkg = this.allActiveAddons.find(pkg => pkg.name === '@embroider/synthesized-styles');
+    if (pkg?.meta['public-assets']) {
+      for (let [filename, appRelativeURL] of Object.entries(pkg.meta['public-assets'] || {})) {
+        let sourcePath = resolvePath(pkg.root, filename);
+        let stats = statSync(sourcePath);
+        assets.push({
+          kind: 'on-disk',
+          sourcePath,
+          relativePath: appRelativeURL,
+          mtime: stats.mtimeMs,
+          size: stats.size,
+        });
       }
     }
 

packages/compat/src/compat-app.ts

@@ -13,7 +13,7 @@ import resolve from 'resolve';
 import { V1Config, WriteV1Config } from './v1-config';
 import { WriteV1AppBoot, ReadV1AppBoot } from './v1-appboot';
 import type { AddonMeta, EmberAppInstance, OutputFileToInputFileMap, PackageInfo } from '@embroider/core';
-import { writeJSONSync, ensureDirSync, copySync, readdirSync, pathExistsSync, existsSync } from 'fs-extra';
+import { writeJSONSync, ensureDirSync, copySync, pathExistsSync, existsSync } from 'fs-extra';
 import AddToTree from './add-to-tree';
 import DummyPackage from './dummy-package';
 import type { TransformOptions } from '@babel/core';
@@ -28,6 +28,7 @@ import { readFileSync } from 'fs';
 import semver from 'semver';
 import type { Transform } from 'babel-plugin-ember-template-compilation';
 import { CompatAppBuilder } from './compat-app-builder';
+import walkSync from 'walk-sync';
 
 interface Group {
   outputFiles: OutputFileToInputFileMap;
@@ -501,7 +502,7 @@ export default class CompatApp {
       };
       let assetPath = join(outputPath, 'assets');
       if (pathExistsSync(assetPath)) {
-        for (let file of readdirSync(assetPath)) {
+        for (let file of walkSync(assetPath, { directories: false })) {
           addonMeta['public-assets']![`./assets/${file}`] = `/assets/${file}`;
         }
       }

packages/vite/src/assets.ts

@@ -3,7 +3,7 @@ import { ResolverLoader } from '@embroider/core';
 import type { Plugin } from 'vite';
 import * as process from 'process';
 import { join, posix } from 'path';
-import { existsSync, readFileSync } from 'fs-extra';
+import { existsSync, readFileSync, lstatSync } from 'fs-extra';
 import send from 'send';
 import type { Readable } from 'stream';
 
@@ -70,9 +70,16 @@ export function assets(): Plugin {
               if (existsSync(join(publicDir, dest))) {
                 return;
               }
+
+              const filePath = join(pkg.root, path);
+              if (!lstatSync(filePath).isFile()) {
+                console.log(`Invalid package definition, ${pkg.name} has defined a file "${path}" that is not a file`);
+                return;
+              }
+
               this.emitFile({
                 type: 'asset',
-                source: readFileSync(join(pkg.root, path)),
+                source: readFileSync(filePath),
                 fileName: posix.resolve('/', dest).slice(1),
               });
             });

tests/app-template/vite.config.mjs

@@ -6,6 +6,7 @@ import {
   templateTag,
   optimizeDeps,
   compatPrebuild,
+  assets,
 } from "@embroider/vite";
 import { resolve } from "path";
 import { babel } from "@rollup/plugin-babel";
@@ -24,6 +25,7 @@ export default defineConfig(({ mode }) => {
       scripts(),
       resolver(),
       compatPrebuild(),
+      assets(),
 
       babel({
         babelHelpers: "runtime",

tests/scenarios/compat-dummy-app-test.ts

@@ -70,26 +70,11 @@ dummyAppScenarios
       throwOnWarnings(hooks);
 
       let app: PreparedApp;
-      let expectFile: ExpectFile;
 
       hooks.before(async () => {
         app = await scenario.prepare();
       });
 
-      test('rewritten app contains public assets from both addon and dummy app after a build', async function (assert) {
-        await app.execute(`pnpm vite build`);
-        expectFile = expectRewrittenFilesAt(resolve(app.dir, 'tests/dummy'), {
-          qunit: assert,
-        });
-        expectFile('../../node_modules/.embroider/rewritten-app/addon-template/from-addon.txt').exists();
-        expectFile('../../node_modules/.embroider/rewritten-app/robots.txt').exists();
-        let assets = expectFile('../../node_modules/.embroider/rewritten-app/package.json')
-          .json()
-          .get('ember-addon.assets');
-        assets.includes('robots.txt');
-        assets.includes('./addon-template/from-addon.txt');
-      });
-
       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 content = readFileSync(`${app.dir}/dist/robots.txt`).toString();

tests/scenarios/compat-stage2-test.ts

@@ -1,8 +1,7 @@
 import type { Options } from '@embroider/compat';
-import { writeFileSync, unlinkSync } from 'fs';
 import type { PreparedApp, Project } from 'scenario-tester';
 import { appScenarios, baseAddon, dummyAppScenarios, renameApp } from './scenarios';
-import { join, resolve } from 'path';
+import { resolve } from 'path';
 import { Rebuilder, Transpiler } from '@embroider/test-support';
 import type { ExpectFile } from '@embroider/test-support/file-assertions/qunit';
 import { expectRewrittenFilesAt } from '@embroider/test-support/file-assertions/qunit';
@@ -659,39 +658,6 @@ stage2Scenarios
         assertFile.matches(/console\.log\(['"]embroider-sample-transforms-result['"]\)/);
       });
 
-      test(`changes in app.css are propagated at rebuild`, async function () {
-        expectFile('assets/my-app.css').doesNotMatch('newly-added-class');
-        writeFileSync(join(app.dir, 'app/styles/app.css'), `.newly-added-class { color: red }`);
-        await builder.build({ changedDirs: [join(app.dir, 'app/styles')] });
-        expectFile('assets/my-app.css').matches('newly-added-class');
-      });
-
-      test(`public assets are included`, async function () {
-        expectFile('public-file-1.txt').matches(/initial state/);
-        expectFile('package.json').json().get('ember-addon.assets').includes('public-file-1.txt');
-      });
-
-      test(`updated public asset`, async function () {
-        writeFileSync(join(app.dir, 'public/public-file-1.txt'), `updated state`);
-        await builder.build({ changedDirs: [join(app.dir, 'app')] });
-
-        expectFile('public-file-1.txt').matches(/updated state/);
-      });
-
-      test(`added public asset`, async function () {
-        writeFileSync(join(app.dir, 'public/public-file-2.txt'), `added`);
-        await builder.build({ changedDirs: [join(app.dir, 'app')] });
-        expectFile('public-file-2.txt').matches(/added/);
-        expectFile('package.json').json().get('ember-addon.assets').includes('public-file-2.txt');
-      });
-
-      test(`removed public asset`, async function () {
-        unlinkSync(join(app.dir, 'public/public-file-1.txt'));
-        await builder.build({ changedDirs: [join(app.dir, 'app')] });
-        expectFile('public-file-1.txt').doesNotExist();
-        expectFile('package.json').json().get('ember-addon.assets').doesNotInclude('public-file-1.txt');
-      });
-
       test('dynamic import is preserved', function () {
         expectFile('./does-dynamic-import.js')
           .transform(build.transpile)