From ce7e6882ac777cf3a259a38600aca2e08158fcdc Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Wed, 22 Jan 2025 17:02:51 -0500 Subject: [PATCH] Fix binary asset support in keepAssets The changes in https://github.com/embroider-build/embroider/pull/2156 broke formatting of binary kept assets, because rollup's default load hook will corrupt them. This provides our own load hook again. But it's still OK to put this plugin after another plugin with a custom load hook, in which case we will take over starting at the `transform` stage instead. In that case, it's up to the other plugin's load hook to produce valid UTF8. --- packages/addon-dev/src/rollup-keep-assets.ts | 34 ++++++++++++++++-- tests/scenarios/v2-addon-dev-test.ts | 37 +++++++++++++++++--- 2 files changed, 63 insertions(+), 8 deletions(-) diff --git a/packages/addon-dev/src/rollup-keep-assets.ts b/packages/addon-dev/src/rollup-keep-assets.ts index 3ea0d5727..29c50d988 100644 --- a/packages/addon-dev/src/rollup-keep-assets.ts +++ b/packages/addon-dev/src/rollup-keep-assets.ts @@ -1,6 +1,7 @@ import type { Plugin } from 'rollup'; import minimatch from 'minimatch'; import { dirname, relative } from 'path'; +import { readFileSync } from 'fs'; // randomly chosen, we're just looking to have high-entropy identifiers that // won't collide with anyting else in the source @@ -15,17 +16,44 @@ export default function keepAssets({ include: string[]; exports?: undefined | 'default' | '*'; }): Plugin { - const marker = `__copy_asset_marker_${counter++}__`; + const marker = `__keep_assets_marker_${counter++}__`; return { - name: 'copy-assets', + name: 'keep-assets', + + // we implement a load hook for the assets we're keeping so that we can + // capture their true binary representations. If we fell through to the + // default rollup load hook we would get utf8 interpretations of them. + // + // Our plugin should be placed after any other plugins that have their own + // load hooks, in which case this will not run but our transform hook will + // still over from there. + load(id: string) { + if (include.some((pattern) => minimatch(id, pattern))) { + return { + code: readFileSync(id).toString('binary'), + meta: { + 'keep-assets': { + binaryLoaded: true, + }, + }, + }; + } + }, transform(code: string, id: string) { + let output: Buffer | string = code; + let ourMeta = this.getModuleInfo(id)?.meta?.['keep-assets']; + if (ourMeta?.binaryLoaded) { + // when the code was produced by our own load hook it is binary-encoded + // string and we can emit the true bytes. + output = Buffer.from(code, 'binary'); + } if (include.some((pattern) => minimatch(id, pattern))) { let ref = this.emitFile({ type: 'asset', fileName: relative(from, id), - source: code, + source: output, }); if (exports === '*') { return `export * from ${marker}("${ref}")`; diff --git a/tests/scenarios/v2-addon-dev-test.ts b/tests/scenarios/v2-addon-dev-test.ts index 36ae7eb24..21b854e54 100644 --- a/tests/scenarios/v2-addon-dev-test.ts +++ b/tests/scenarios/v2-addon-dev-test.ts @@ -1,10 +1,11 @@ -import path from 'path'; +import path, { resolve } from 'path'; import { appScenarios, baseV2Addon } from './scenarios'; import { PreparedApp, type Project } from 'scenario-tester'; import QUnit from 'qunit'; import merge from 'lodash/merge'; import type { ExpectFile } from '@embroider/test-support/file-assertions/qunit'; import { expectFilesAt } from '@embroider/test-support/file-assertions/qunit'; +import { writeFileSync } from 'fs'; const { module: Qmodule, test } = QUnit; @@ -85,11 +86,7 @@ appScenarios addon.gjs(), addon.dependencies(), addon.publicAssets('public'), - addon.keepAssets(["**/*.css"]), - // this works with custom-asset plugin below to exercise whether we can keepAssets - // for generated files that have exports - addon.keepAssets(["**/*.xyz"], "default"), { name: 'virtual-css', resolveId(source, importer) { @@ -116,6 +113,12 @@ appScenarios } } }, + + addon.keepAssets(["**/*.css"]), + + // this works with custom-asset plugin above to exercise whether we can keepAssets + // for generated files that have exports + addon.keepAssets(["**/*.{xyz,png}"], "default"), ], }; `, @@ -205,6 +208,12 @@ appScenarios return value; } `, + 'has-binary-import.js': ` + import helloURL from './hello.png'; + export default function() { + return helloURL; + } + `, }, }, public: { @@ -335,6 +344,13 @@ appScenarios assert.dom().containsText('namespaced component'); }); + + test('valid image asset is kept in addon', async function (assert) { + let module = await import('v2-addon/asset-examples/has-binary-import'); + this.url = module.default(); + await render(hbs\`hello\`); + assert.dom('img').hasStyle({'width': '30px'}); + }) }); `, 'asset-test.js': ` @@ -391,6 +407,17 @@ appScenarios hooks.before(async () => { app = await scenario.prepare(); + // fixturify (via scenario-tester) has no binary output support + let v2AddonPath = path.dirname(require.resolve(`v2-addon/package.json`, { paths: [app.dir] })); + console.log(`v2addonpath`, v2AddonPath); + console.log(`resolved`, resolve(v2AddonPath, 'src/asset-examples/hello.png')); + writeFileSync( + resolve(v2AddonPath, 'src/asset-examples/hello.png'), + Buffer.from( + 'iVBORw0KGgoAAAANSUhEUgAAAB4AAAALCAYAAABoKz2KAAACYElEQVQ4y8WU3UuTYRjGf8+77d2XuplL5vdMI0QzwhTRLEgUEqIQwg76Oo7ypD+goIPO60Q6DTq2giAKAiHNvoaRijNDrYk606Fuc9v7Pk8Hs+k6CI/ygvvkgvu5bq7rfm5RVVWl2Ado7BM0gNZqPx6HTnd9gOPlvn83CMGVtgb6u5qpPegBwG7RuNHZxK2uZlw2696Eyzxurp9pRhPQUltBfXkxhlTs9t9UClNmGKUUE+EINf4iAj4vAIZUfJlfoqHSj1vfEVYK0n+99QfW3uY6pheWWUukAKir8PPwWiWfZ37yePgrpw6X0dNUhyYEg+/HGZ5Z4NPcEn1pI2ew0e8L9LU1Zjm3buVmdwv+wgJC4WUevQliqp0RtKOBUp5+mMwS8WSKJ0NBTtZVU+jUuXy6iZGpOUan57nUfgybZW9r0XviCF6Xk4GX72gMlNJaU5JrdfhXlJmV9SwxH1kjtLiKpgm8Ljv5DjtNNeU0BkqJxrewamJPwsWefL4trjC1HGU5ukFt8YGM/dtlffZxMicDtcuOrZRBPJVm4NUo4WicQqdOIm1mcjUlvnwXQmSyVEohpaLEm8dKbIvNRBJfvhuLJshz2lmNJQC4c6ED3WbFGvwRyQpJJZEqswxSKjZTBkPjM9y92EkybRBZj3FvcAilYCQ0x/mWBg75i7j//C2GVIzNLtDf087YbJgXwRC3z3Xw4OpZEimD1xOzCKDA5UC32RC7D4ghFZrIfJmkKbFv52nRNAqcOpGNRI7VSVMiBOialrUxaUosQmDTBKYCn9vBSiyBRWT6UlKCIlf4f+I3ZibyifAuOoEAAAAASUVORK5CYII=', + 'base64' + ) + ); let result = await inDependency(app, 'v2-addon').execute('pnpm build'); if (result.exitCode !== 0) { throw new Error(result.output);