fix(webpack): include hash in asset filenames so they do not conflict (#27159)

Our assets are generated as flat assets in dist, which allows using
assets from workspace libs. This prevents users from having different
assets with the same filename (e.g. `foo/image.png` and
`bar/image.png`). This will error out in the dev server with conflicting
filenames.

We cannot use `[path][name]` because of assets that are outside of the
app folder (e.g. `../../libs/ui/src/assets/image.png`). Thus the best
option is to include hash.

Note: Also re-enabled the e2e tests for `react.test.ts` file since it is
now using Playwright instead of Cypress.

## Current Behavior
Assets with the same filename will error in dev-mode.

## Expected Behavior
Assets with the same filename works.

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #18272
This commit is contained in:
Jack Hsu 2024-07-29 09:47:33 -04:00 committed by GitHub
parent c02f254316
commit 0710feab27
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 24 additions and 24 deletions

View File

@ -68,6 +68,8 @@ describe('React Applications', () => {
const libName = uniq('lib'); const libName = uniq('lib');
const libWithNoComponents = uniq('lib'); const libWithNoComponents = uniq('lib');
const logoSvg = readFileSync(join(__dirname, 'logo.svg')).toString(); const logoSvg = readFileSync(join(__dirname, 'logo.svg')).toString();
const blueSvg = `<?xml version="1.0"?><svg xmlns="http://www.w3.org/2000/svg" version="1.2" baseProfile="tiny" viewBox="0 0 30 30"><rect x="10" y="10" width="10" height="10" fill="blue"/></svg>`;
const redSvg = `<?xml version="1.0"?><svg xmlns="http://www.w3.org/2000/svg" version="1.2" baseProfile="tiny" viewBox="0 0 30 30"><rect x="10" y="10" width="10" height="10" fill="red"/></svg>`;
runCLI( runCLI(
`generate @nx/react:app ${appName} --style=css --bundler=webpack --no-interactive --skipFormat` `generate @nx/react:app ${appName} --style=css --bundler=webpack --no-interactive --skipFormat`
@ -92,11 +94,15 @@ describe('React Applications', () => {
` `
); );
updateFile(`apps/${appName}/src/app/blue/img.svg`, blueSvg); // ensure that same filenames do not conflict
updateFile(`apps/${appName}/src/app/red/img.svg`, redSvg); // ensure that same filenames do not conflict
updateFile(`apps/${appName}/src/app/logo.svg`, logoSvg); updateFile(`apps/${appName}/src/app/logo.svg`, logoSvg);
updateFile( updateFile(
`apps/${appName}/src/app/app.tsx`, `apps/${appName}/src/app/app.tsx`,
` `
import { ReactComponent as Logo } from './logo.svg'; import { ReactComponent as Logo } from './logo.svg';
import blue from './blue/img.svg';
import red from './red/img.svg';
import logo from './logo.svg'; import logo from './logo.svg';
import NxWelcome from './nx-welcome'; import NxWelcome from './nx-welcome';
@ -105,6 +111,8 @@ describe('React Applications', () => {
<> <>
<Logo /> <Logo />
<img src={logo} alt="" /> <img src={logo} alt="" />
<img src={blue} alt="" />
<img src={red} alt="" />
<NxWelcome title="${appName}" /> <NxWelcome title="${appName}" />
</> </>
); );
@ -134,10 +142,7 @@ describe('React Applications', () => {
checkSourceMap: true, checkSourceMap: true,
checkStyles: true, checkStyles: true,
checkLinter: true, checkLinter: true,
// TODO(caleb): Fix cypress tests // TODO(jack): check why Playwright tests are timing out in CI
// /tmp/nx-e2e--1970-rQ4U0qBe6Nht/nx/proj1614306/dist/apps/app5172641/server/runtime.js:119
// if (typeof import.meta.url === "string") scriptUrl = import.meta.url
// SyntaxError: Cannot use 'import.meta' outside a module
checkE2E: false, checkE2E: false,
}); });
@ -150,13 +155,10 @@ describe('React Applications', () => {
checkSourceMap: false, checkSourceMap: false,
checkStyles: false, checkStyles: false,
checkLinter: false, checkLinter: false,
// TODO(caleb): Fix cypress tests // TODO(jack): check why Playwright tests are timing out in CI
// /tmp/nx-e2e--1970-rQ4U0qBe6Nht/nx/proj1614306/dist/apps/app5172641/server/runtime.js:119
// if (typeof import.meta.url === "string") scriptUrl = import.meta.url
// SyntaxError: Cannot use 'import.meta' outside a module
checkE2E: false, checkE2E: false,
}); });
}, 500000); }, 500_000);
it('should generate app with routing', async () => { it('should generate app with routing', async () => {
const appName = uniq('app'); const appName = uniq('app');
@ -532,7 +534,7 @@ async function testGeneratedApp(
if (opts.checkE2E && runE2ETests()) { if (opts.checkE2E && runE2ETests()) {
const e2eResults = runCLI(`e2e ${appName}-e2e`); const e2eResults = runCLI(`e2e ${appName}-e2e`);
expect(e2eResults).toContain('All specs passed!'); expect(e2eResults).toContain('Successfully ran target e2e');
expect(await killPorts()).toBeTruthy(); expect(await killPorts()).toBeTruthy();
} }
} }

View File

@ -5,6 +5,7 @@ import {
createFile, createFile,
isNotWindows, isNotWindows,
killPorts, killPorts,
listFiles,
newProject, newProject,
readFile, readFile,
runCLI, runCLI,
@ -79,22 +80,27 @@ describe('Web Components Applications', () => {
'none' 'none'
); );
runCLI(`build ${appName}`); runCLI(`build ${appName}`);
const images = listFiles(`dist/apps/${appName}`).filter((f) =>
f.endsWith('.png')
);
checkFilesExist( checkFilesExist(
`dist/apps/${appName}/index.html`, `dist/apps/${appName}/index.html`,
`dist/apps/${appName}/runtime.js`, `dist/apps/${appName}/runtime.js`,
`dist/apps/${appName}/emitted.png`,
`dist/apps/${appName}/main.js`, `dist/apps/${appName}/main.js`,
`dist/apps/${appName}/styles.css` `dist/apps/${appName}/styles.css`
); );
checkFilesDoNotExist(`dist/apps/${appName}/inlined.png`); expect(images.some((f) => f.startsWith('emitted.'))).toBe(true);
expect(images.some((f) => f.startsWith('inlined.'))).toBe(false);
expect(readFile(`dist/apps/${appName}/main.js`)).toContain( expect(readFile(`dist/apps/${appName}/main.js`)).toContain(
'data:image/png;base64' 'data:image/png;base64'
); );
// Should not be a JS module but kept as a PNG // Should not be a JS module but kept as a PNG
expect(readFile(`dist/apps/${appName}/emitted.png`)).not.toContain( expect(
'export default' readFile(
); `dist/apps/${appName}/${images.find((f) => f.startsWith('emitted.'))}`
)
).not.toContain('export default');
expect(readFile(`dist/apps/${appName}/index.html`)).toContain( expect(readFile(`dist/apps/${appName}/index.html`)).toContain(
'<link rel="stylesheet" href="styles.css">' '<link rel="stylesheet" href="styles.css">'

View File

@ -332,6 +332,7 @@ export function applyWebConfig(
config.output = { config.output = {
...config.output, ...config.output,
assetModuleFilename: '[name].[contenthash:20][ext]',
crossOriginLoading: options.subresourceIntegrity crossOriginLoading: options.subresourceIntegrity
? ('anonymous' as const) ? ('anonymous' as const)
: (false as const), : (false as const),
@ -404,9 +405,6 @@ export function applyWebConfig(
maxSize: 10_000, // 10 kB maxSize: 10_000, // 10 kB
}, },
}, },
generator: {
filename: `[name]${hashFormat.file}[ext]`,
},
}, },
// SVG: same as image but we need to separate it so it can be swapped for SVGR in the React plugin. // SVG: same as image but we need to separate it so it can be swapped for SVGR in the React plugin.
{ {
@ -417,17 +415,11 @@ export function applyWebConfig(
maxSize: 10_000, // 10 kB maxSize: 10_000, // 10 kB
}, },
}, },
generator: {
filename: `[name]${hashFormat.file}[ext]`,
},
}, },
// Fonts: Emit separate file and export the URL. // Fonts: Emit separate file and export the URL.
{ {
test: /\.(eot|otf|ttf|woff|woff2)$/, test: /\.(eot|otf|ttf|woff|woff2)$/,
type: 'asset/resource', type: 'asset/resource',
generator: {
filename: `[name]${hashFormat.file}[ext]`,
},
}, },
...rules, ...rules,
], ],