Skip to content

Commit

Permalink
fix(webpack): bring back previous SVG and SVGR behavior for React pro…
Browse files Browse the repository at this point in the history
…jects
  • Loading branch information
jaysoo committed Apr 2, 2024
1 parent 1a82dd2 commit 517ecb8
Show file tree
Hide file tree
Showing 9 changed files with 224 additions and 81 deletions.
80 changes: 80 additions & 0 deletions e2e/next-extensions/src/next-svgr.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import {
cleanupProject,
createFile,
listFiles,
newProject,
readFile,
runCLI,
runE2ETests,
uniq,
updateFile,
} from '@nx/e2e/utils';

describe('NextJs SVGR support', () => {
beforeAll(() => {
newProject({
packages: ['@nx/next'],
});
});

afterAll(() => cleanupProject());

it('should allow both SVG asset and SVGR component to be used', () => {
const appName = uniq('app');
runCLI(
`generate @nx/next:app ${appName} --no-interactive --appDir=true --src=true`
);
createFile(
`apps/${appName}/src/app/nx.svg`,
`
<svg version="1.1" width="300" height="200" xmlns="http://www.w3.org/2000/svg">
<text x="150" y="125" font-size="60" text-anchor="middle" fill="white">SVG for app</text>
</svg>
`
);
updateFile(
`apps/${appName}/src/app/page.tsx`,
`
import Image from 'next/image';
import svgImg, { ReactComponent as Logo } from './nx.svg';
export default async function Index() {
return (
<>
<Image src={svgImg} alt="Alt for SVG img tag" />
<Logo />
</>
);
}
`
);
updateFile(
`apps/${appName}/next.config.js`,
`
const { composePlugins, withNx } = require('@nx/next');
const nextConfig = {
nx: {
svgr: true,
},
};
const plugins = [
withNx,
];
module.exports = composePlugins(...plugins)(nextConfig);
`
);

runCLI(`build ${appName}`);

const pageFile = readFile(`apps/${appName}/.next/server/app/page.js`);
const svgFile = listFiles(`apps/${appName}/.next/static/media`).find((f) =>
/nx\.[a-z0-9]+\.svg$/.test(f)
);
expect(`apps/${appName}/.next/static/chunks/app/${pageFile}`).toMatch(
/SVG for app/
);
expect(`apps/${appName}/.next/static/chunks/app/${pageFile}`).toMatch(
/Alt for SVG img tag/
);
expect(svgFile).toBeTruthy();
});
});
65 changes: 65 additions & 0 deletions e2e/react-extensions/src/react-webpack.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import {
cleanupProject,
createFile,
listFiles,
newProject,
readFile,
runCLI,
runCLIAsync,
uniq,
updateFile,
} from '@nx/e2e/utils';

describe('Build React applications and libraries with Vite', () => {
beforeAll(() => {
newProject({
packages: ['@nx/react'],
});
});

afterAll(() => {
cleanupProject();
});

// Regression test: https://github.com/nrwl/nx/issues/21773
it('should support SVGR and SVG asset in the same project', async () => {
const appName = uniq('app');

runCLI(
`generate @nx/react:app ${appName} --bundler=webpack --compiler=babel --unitTestRunner=none --no-interactive`
);
createFile(
`apps/${appName}/src/app/nx.svg`,
`
<svg version="1.1" width="300" height="200" xmlns="http://www.w3.org/2000/svg">
<text x="150" y="125" font-size="60" text-anchor="middle" fill="white">SVG for app</text>
</svg>
`
);
updateFile(
`apps/${appName}/src/app/app.tsx`,
`
import svgImg, { ReactComponent as Logo } from './nx.svg';
export function App() {
return (
<>
<img src={svgImg} alt="Alt for SVG img tag" />
<Logo />
</>
);
}
export default App;
`
);

await runCLIAsync(`build ${appName}`);

const outFiles = listFiles(`dist/apps/${appName}`);
const mainFile = outFiles.find((f) => f.startsWith('main.'));
const mainContent = readFile(`dist/apps/${appName}/${mainFile}`);
const svgFile = outFiles.find((f) => f.endsWith('.svg'));
expect(mainContent).toMatch(/SVG for app/);
expect(mainContent).toMatch(/Alt for SVG img tag/);
expect(svgFile).toBeTruthy();
}, 300_000);
});
1 change: 1 addition & 0 deletions packages/next/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
"webpack",
// require.resovle is used for these
"@babel/plugin-proposal-decorators",
"file-loader",
"url-loader",
"@svgr/webpack"
]
Expand Down
1 change: 1 addition & 0 deletions packages/next/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
"@svgr/webpack": "^8.0.1",
"chalk": "^4.1.0",
"copy-webpack-plugin": "^10.2.4",
"file-loader": "^6.2.0",
"fs-extra": "^11.1.0",
"ignore": "^5.0.4",
"semver": "^7.5.3",
Expand Down
67 changes: 1 addition & 66 deletions packages/next/plugins/with-nx.spec.ts
Original file line number Diff line number Diff line change
@@ -1,69 +1,4 @@
import { NextConfigComplete } from 'next/dist/server/config-shared';
import { getAliasForProject, getNextConfig } from './with-nx';

describe('getNextConfig', () => {
describe('svgr', () => {
it('should be used by default', () => {
const config = getNextConfig();

const result = config.webpack(
{
module: { rules: [{ oneOf: [] }] },
},
{
buildId: 'build-id',
config: config as NextConfigComplete,
dev: true,
dir: 'dir',
isServer: false,
totalPages: 0,
webpack: undefined,
defaultLoaders: {
babel: {
options: {},
},
},
}
);

expect(
result.module.rules.some((rule) => rule.test?.test('cat.svg'))
).toBe(true);
});

it('should not be used when disabled', () => {
const config = getNextConfig({
nx: {
svgr: false,
},
});

const result = config.webpack(
{
module: { rules: [{ oneOf: [] }] },
},
{
buildId: 'build-id',
config: config as NextConfigComplete,
dev: true,
dir: 'dir',
isServer: false,
totalPages: 0,
webpack: undefined,
defaultLoaders: {
babel: {
options: {},
},
},
}
);

expect(
result.module.rules.some((rule) => rule.test?.test('cat.svg'))
).toBe(false);
});
});
});
import { getAliasForProject } from './with-nx';

describe('getAliasForProject', () => {
it('should return the matching alias for a project', () => {
Expand Down
63 changes: 48 additions & 15 deletions packages/next/plugins/with-nx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -337,22 +337,55 @@ export function getNextConfig(

// Default SVGR support to be on for projects.
if (nx?.svgr !== false) {
config.module.rules.push(
// Apply rule for svg imports ending in ?url
{
test: /\.svg$/i,
type: 'asset',
resourceQuery: /url/, // apply to *.svg?url
// TODO(v20): Remove file-loader and use `?react` querystring to differentiate between asset and SVGR.
// It should be:
// use: [{
// test: /\.svg$/i,
// type: 'asset',
// resourceQuery: /react/, // *.svg?react
// },
// {
// test: /\.svg$/i,
// issuer: /\.[jt]sx?$/,
// resourceQuery: { not: [/react/] }, // exclude react component if *.svg?react
// use: ['@svgr/webpack'],
// }],
// See:
// - SVGR: https://react-svgr.com/docs/webpack/#use-svgr-and-asset-svg-in-the-same-project
// - Vite: https://www.npmjs.com/package/vite-plugin-svgr
// - Rsbuild: https://github.com/web-infra-dev/rsbuild/pull/1783
// Note: We also need a migration for any projects that are using SVGR to convert
// `import { ReactComponent as X } from './x.svg` to
// `import X from './x.svg?react';
config.module.rules.push({
test: /\.svg$/,
issuer: { not: /\.(css|scss|sass)$/ },
resourceQuery: {
not: [
/__next_metadata__/,
/__next_metadata_route__/,
/__next_metadata_image_meta__/,
],
},

// Convert all other svg imports to React components
{
test: /\.svg$/i,
issuer: /\.[jt]sx?$/,
resourceQuery: { not: [/url/] },
use: ['@svgr/webpack'],
}
);
use: [
{
loader: require.resolve('@svgr/webpack'),
options: {
svgo: false,
titleProp: true,
ref: true,
},
},
{
loader: require.resolve('file-loader'),
options: {
// Next.js hard-codes assets to load from "static/media".
// See: https://github.com/vercel/next.js/blob/53d017d/packages/next/src/build/webpack-config.ts#L1993
name: 'static/media/[name].[hash].[ext]',
},
},
],
});
}

return userWebpack(config, options);
Expand Down
1 change: 1 addition & 0 deletions packages/react/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
"babel-plugin-emotion",
"babel-plugin-styled-components",
"css-loader",
"file-loader",
"less-loader",
"react-refresh",
"rollup",
Expand Down
1 change: 1 addition & 0 deletions packages/react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"@phenomnomnominal/tsquery": "~5.0.1",
"@svgr/webpack": "^8.0.1",
"chalk": "^4.1.0",
"file-loader": "^6.2.0",
"minimatch": "9.0.3",
"tslib": "^2.3.0",
"@nx/devkit": "file:../devkit",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,26 @@ export function applyReactConfig(
if (options.svgr !== false) {
removeSvgLoaderIfPresent(config);

// TODO(v20): Remove file-loader and use `?react` querystring to differentiate between asset and SVGR.
// It should be:
// use: [{
// test: /\.svg$/i,
// type: 'asset',
// resourceQuery: /react/, // *.svg?react
// },
// {
// test: /\.svg$/i,
// issuer: /\.[jt]sx?$/,
// resourceQuery: { not: [/react/] }, // exclude react component if *.svg?react
// use: ['@svgr/webpack'],
// }],
// See:
// - SVGR: https://react-svgr.com/docs/webpack/#use-svgr-and-asset-svg-in-the-same-project
// - Vite: https://www.npmjs.com/package/vite-plugin-svgr
// - Rsbuild: https://github.com/web-infra-dev/rsbuild/pull/1783
// Note: We also need a migration for any projects that are using SVGR to convert
// `import { ReactComponent as X } from './x.svg` to
// `import X from './x.svg?react';
config.module.rules.push({
test: /\.svg$/,
issuer: /\.(js|ts|md)x?$/,
Expand All @@ -23,6 +43,12 @@ export function applyReactConfig(
ref: true,
},
},
{
loader: require.resolve('file-loader'),
options: {
name: '[name].[hash].[ext]',
},
},
],
});
}
Expand Down

0 comments on commit 517ecb8

Please sign in to comment.