Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NO_JIRA: Extract and optimise JS Loaders #178

Merged
merged 2 commits into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 32 additions & 2 deletions packages/react-scripts/backpack-addons/cssModules.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
'use strict';

// eslint-disable-next-line no-unused-vars
const postcssNormalize = require('postcss-normalize');
const paths = require('../config/paths');
const appPackageJson = require(paths.appPackageJson);
const bpkReactScriptsConfig = appPackageJson['backpack-react-scripts'] || {};
const cssModulesEnabled = bpkReactScriptsConfig.cssModules !== false;
const noBackpackStylePrefixes =
bpkReactScriptsConfig.danger_noBackpackStylePrefixes !== false;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Early tests show that we can live without these. Now I'd like to do more tests under this flag


// Backpack node module regexes
const backpackModulesRegex = /node_modules[\\/]bpk-/;
Expand All @@ -22,6 +22,12 @@ const getStyleTestRegexes = regexType => {

switch (regexType) {
case 'css':
if (noBackpackStylePrefixes) {
return {
and: [cssRegex, () => !cssModulesEnabled],
};
}

return {
and: [cssRegex, () => !cssModulesEnabled],
not: [
Expand All @@ -31,6 +37,15 @@ const getStyleTestRegexes = regexType => {
],
};
case 'cssModule':
if (noBackpackStylePrefixes) {
return [
cssModuleRegex,
{
and: [cssRegex, () => cssModulesEnabled],
},
];
}

return [
cssModuleRegex,
{
Expand All @@ -46,6 +61,12 @@ const getStyleTestRegexes = regexType => {
},
];
case 'sass':
if (noBackpackStylePrefixes) {
return {
and: [sassRegex, () => !cssModulesEnabled],
};
}

return {
and: [sassRegex, () => !cssModulesEnabled],
not: [
Expand All @@ -55,6 +76,15 @@ const getStyleTestRegexes = regexType => {
],
};
case 'sassModule':
if (noBackpackStylePrefixes) {
return [
sassModuleRegex,
{
and: [sassRegex, () => cssModulesEnabled],
},
];
}

return [
sassModuleRegex,
{
Expand Down
72 changes: 72 additions & 0 deletions packages/react-scripts/backpack-addons/useTsChecker.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
'use strict';

const ForkTsCheckerWebpackPlugin =
process.env.TSC_COMPILE_ON_ERROR === 'true'
? require('react-dev-utils/ForkTsCheckerWarningWebpackPlugin')
: require('react-dev-utils/ForkTsCheckerWebpackPlugin');

const paths = require('../config/paths');
const appPackageJson = require(paths.appPackageJson);
const resolve = require('resolve');
const bpkReactScriptsConfig = appPackageJson['backpack-react-scripts'] || {};
const skipProductionTypeCheck =
bpkReactScriptsConfig.skipProductionTypeCheck || false;

const useTsChecker = (
Copy link
Author

@xalechez xalechez Nov 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skipping TS Checker plugin results in around 20-30% build time reduction as early tests show. tsc can do the same job better and faster. I've made a new flag that is totally opt-in for consumers and keep type checking during development

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used this plugin to measure loaders and plugins speed

isEnvDevelopment,
isEnvProduction,
shouldUseSourceMap
) => {
if (skipProductionTypeCheck && isEnvProduction) {
return;
}

return new ForkTsCheckerWebpackPlugin({
async: isEnvDevelopment,
typescript: {
typescriptPath: resolve.sync('typescript', {
basedir: paths.appNodeModules,
}),
configOverwrite: {
compilerOptions: {
sourceMap: isEnvProduction ? shouldUseSourceMap : isEnvDevelopment,
skipLibCheck: true,
inlineSourceMap: false,
declarationMap: false,
noEmit: true,
incremental: true,
tsBuildInfoFile: paths.appTsBuildInfoFile,
},
},
context: paths.appPath,
diagnosticOptions: {
syntactic: true,
},
mode: 'write-references',
// profile: true,
},
issue: {
// This one is specifically to match during CI tests,
// as micromatch doesn't match
// '../cra-template-typescript/template/src/App.tsx'
// otherwise.
include: [
{ file: '../**/src/**/*.{ts,tsx}' },
{ file: '**/src/**/*.{ts,tsx}' },
],
exclude: [
{ file: '**/src/**/__tests__/**' },
{ file: '**/src/**/?(*.){spec|test}.*' },
{ file: '**/src/setupProxy.*' },
{ file: '**/src/setupTests.*' },
],
},
logger: {
infrastructure: 'silent',
},
});
};

module.exports = {
useTsChecker,
};
69 changes: 69 additions & 0 deletions packages/react-scripts/config/loaders/getAssetLoaders.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
'use strict';
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is identical in two configs. Extracting helps to avoid undesired mismatch


const imageInlineSizeLimit = parseInt(
process.env.IMAGE_INLINE_SIZE_LIMIT || '10000'
);

function getAssetLoaders() {
return [
// TODO: Merge this config once `image/avif` is in the mime-db
// https://github.com/jshttp/mime-db
{
test: [/\.avif$/],
type: 'asset',
mimetype: 'image/avif',
parser: {
dataUrlCondition: {
maxSize: imageInlineSizeLimit,
},
},
},
{
test: [/\.bmp$/, /\.gif$/, /\.jpe?g$/, /\.png$/],
type: 'asset',
parser: {
dataUrlCondition: {
maxSize: imageInlineSizeLimit,
},
},
},
{
test: /\.svg$/,
use: [
{
loader: require.resolve('@svgr/webpack'),
options: {
prettier: false,
svgoConfig: {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed comments from #177

plugins: [
{
name: 'preset-default',
params: {
overrides: {
removeViewBox: false,
},
},
},
],
},
titleProp: true,
ref: true,
},
},
{
loader: require.resolve('file-loader'),
options: {
name: 'static/media/[name].[contenthash:8].[ext]',
},
},
],
issuer: {
and: [/\.(ts|tsx|js|jsx|md|mdx)$/],
},
},
];
}

module.exports = {
getAssetLoaders,
};
103 changes: 103 additions & 0 deletions packages/react-scripts/config/loaders/getJSLoaders.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
'use strict';
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is identical in two configs. Extracting helps to avoid undesired mismatch


const withIncludedPrefixes = require('../../backpack-addons/babelIncludePrefixes');
const getCacheIdentifier = require('react-dev-utils/getCacheIdentifier');

function getJSLoaders(
isEnvProduction,
isEnvDevelopment,
shouldUseReactRefresh,
hasJsxRuntime,
shouldUseSourceMap
) {
return [
// Process application JS with Babel.
// The preset includes JSX, Flow, TypeScript, and some ESnext features.
{
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed the second round of Babel which was unnecessary

test: /\.(js|mjs|cjs|jsx|ts|tsx)$/,
include: withIncludedPrefixes(), // #backpack-addons babelIncludePrefixes
loader: require.resolve('babel-loader'),
options: {
customize: require.resolve('babel-preset-react-app/webpack-overrides'),
presets: [
[
require.resolve('babel-preset-react-app'),
{
runtime: hasJsxRuntime ? 'automatic' : 'classic',
},
],
],
// @remove-on-eject-begin
babelrc: false,
configFile: false,
// Make sure we have a unique cache identifier, erring on the
// side of caution.
// We remove this when the user ejects because the default
// is sane and uses Babel options. Instead of options, we use
// the react-scripts and babel-preset-react-app versions.
cacheIdentifier: getCacheIdentifier(
isEnvProduction ? 'production' : isEnvDevelopment && 'development',
[
'babel-plugin-named-asset-import',
'babel-preset-react-app',
'react-dev-utils',
'react-scripts',
]
),
// @remove-on-eject-end
plugins: [
require.resolve('@loadable/babel-plugin'),
isEnvDevelopment &&
shouldUseReactRefresh &&
require.resolve('react-refresh/babel'),
].filter(Boolean),
// This is a feature of `babel-loader` for webpack (not Babel itself).
// It enables caching results in ./node_modules/.cache/babel-loader/
// directory for faster rebuilds.
cacheDirectory: true,
// See #6846 for context on why cacheCompression is disabled
cacheCompression: false,
compact: isEnvProduction,
},
},
{
test: /\.(js|mjs|cjs)$/,
exclude: [/@babel(?:\/|\\{1,2})runtime/, ...withIncludedPrefixes()],
loader: require.resolve('babel-loader'),
options: {
babelrc: false,
configFile: false,
compact: false,
presets: [
[
require.resolve('babel-preset-react-app/dependencies'),
{ helpers: true },
],
],
cacheDirectory: true,
// See #6846 for context on why cacheCompression is disabled
cacheCompression: false,
// @remove-on-eject-begin
cacheIdentifier: getCacheIdentifier(
isEnvProduction ? 'production' : isEnvDevelopment && 'development',
[
'babel-plugin-named-asset-import',
'babel-preset-react-app',
'react-dev-utils',
'react-scripts',
]
),
// @remove-on-eject-end
// Babel sourcemaps are needed for debugging into node_modules
// code. Without the options below, debuggers like VSCode
// show incorrect code and set breakpoints on the wrong lines.
sourceMaps: shouldUseSourceMap,
inputSourceMap: shouldUseSourceMap,
},
},
];
}

module.exports = {
getJSLoaders,
};
Loading