Skip to content

Commit

Permalink
Fix binary asset support in keepAssets
Browse files Browse the repository at this point in the history
The changes in #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.
  • Loading branch information
ef4 committed Jan 22, 2025
1 parent d346bd1 commit ce7e688
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 8 deletions.
34 changes: 31 additions & 3 deletions packages/addon-dev/src/rollup-keep-assets.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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}")`;
Expand Down
37 changes: 32 additions & 5 deletions tests/scenarios/v2-addon-dev-test.ts
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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) {
Expand All @@ -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"),
],
};
`,
Expand Down Expand Up @@ -205,6 +208,12 @@ appScenarios
return value;
}
`,
'has-binary-import.js': `
import helloURL from './hello.png';
export default function() {
return helloURL;
}
`,
},
},
public: {
Expand Down Expand Up @@ -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\`<img alt="hello" src={{this.url}}>\`);
assert.dom('img').hasStyle({'width': '30px'});
})
});
`,
'asset-test.js': `
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit ce7e688

Please sign in to comment.