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

Remove require from UMD bundles #12485

Merged
merged 15 commits into from Mar 23, 2022
1 change: 1 addition & 0 deletions .eslintignore
@@ -1,3 +1,4 @@
.tmp
!/*.js
/tests/format/**/*.js
/tests/integration/cli/
Expand Down
6 changes: 5 additions & 1 deletion .eslintrc.js
Expand Up @@ -167,7 +167,11 @@ module.exports = {
},
},
{
files: ["**/*.mjs", "scripts/release/**/*.js"],
files: [
"**/*.mjs",
"scripts/release/**/*.js",
"scripts/tools/bundle-test/**/*.js",
],
parserOptions: {
sourceType: "module",
},
Expand Down
61 changes: 61 additions & 0 deletions .github/workflows/bundler-friendly.yml
@@ -0,0 +1,61 @@
name: Bundler_Friendly

on:
schedule:
# “At 00:00 on Sunday.” https://crontab.guru/#0%C2%A00%C2%A0*%C2%A0*%C2%A00
- cron: "0 0 * * 0"
pull_request:
paths:
# This workflow file
- ".github/workflows/bundler-friendly.yml"

jobs:
build:
name: Build
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v3

- name: Setup Node.js
uses: actions/setup-node@v3
with:
cache: "yarn"

- name: Install Dependencies
run: yarn install --frozen-lockfile

- name: Build Package
run: yarn build

- name: Upload Artifact
uses: actions/upload-artifact@v2
with:
name: dist
path: dist

webpack:
name: Bundle Prettier with webpack
runs-on: ubuntu-latest
needs: [build]
defaults:
run:
working-directory: scripts/tools/bundle-test
steps:
- name: Checkout
uses: actions/checkout@v2.4.0

- name: Setup Node.js
uses: actions/setup-node@v2.5.1

- name: Download Artifact
uses: actions/download-artifact@v2
with:
name: dist
path: dist

- name: Install Dependencies
run: yarn install --frozen-lockfile

- name: Test
run: yarn test
6 changes: 4 additions & 2 deletions .github/workflows/dev-package-test.yml
Expand Up @@ -2,12 +2,14 @@ name: Dev_Package_Test

on:
schedule:
- cron: "0 0 * * 1"
# “At 00:00 on Sunday.” https://crontab.guru/#0%C2%A00%C2%A0*%C2%A0*%C2%A00
- cron: "0 0 * * 0"
pull_request:
paths:
- "package.json"
- ".github/workflows/dev-package-test.yml"
- "yarn.lock"
# This workflow file
- ".github/workflows/dev-package-test.yml"

jobs:
test:
Expand Down
10 changes: 7 additions & 3 deletions .gitignore
@@ -1,12 +1,17 @@
/.cache
# node_modules
# We have `node_modules` directory for tests, don't ignore them all
/node_modules
/website/node_modules
/scripts/release/node_modules
/scripts/tools/bundle-test/node_modules
/scripts/tools/eslint-plugin-prettier-internal-rules/node_modules

.tmp
*.log
/errors
/test*.*
/.vscode
/dist
/website/node_modules
/website/build
/website/i18n
/website/static/playground.js
Expand All @@ -22,5 +27,4 @@ package-lock.json
!.yarn/versions
.pnp.*
.nyc_output
/scripts/tools/eslint-plugin-prettier-internal-rules/node_modules
.devcontainer
1 change: 1 addition & 0 deletions .prettierignore
@@ -1,3 +1,4 @@
.tmp
dist/
.cache/
coverage/
Expand Down
3 changes: 3 additions & 0 deletions changelog_unreleased/misc/12485.md
@@ -0,0 +1,3 @@
#### Make artifact friendly for `webpack` (#12485 by @fisker)

Previously, when bundling our UMD files `standalone.js`, `parser-typescript.js`, `webpack` warn about "Critical dependency: the request of a dependency is an expression", now this is fixed.
26 changes: 26 additions & 0 deletions scripts/build/config.mjs
@@ -1,6 +1,7 @@
import path from "node:path";
import { createRequire } from "node:module";
import createEsmUtils from "esm-utils";
import { PROJECT_ROOT } from "../utils/index.mjs";

const { require } = createEsmUtils(import.meta);

Expand Down Expand Up @@ -92,6 +93,24 @@ const parsers = [
find: "typescriptVersionIsAtLeast[version] = semverCheck(version);",
replacement: "typescriptVersionIsAtLeast[version] = true;",
},
// The next two replacement fixed webpack warning `Critical dependency: require function is used in a way in which dependencies cannot be statically extracted`
// #12338
{
file: require.resolve(
"@typescript-eslint/typescript-estree/dist/create-program/shared.js"
),
find: "moduleResolver = require(moduleResolverPath);",
replacement: "throw new Error('Dynamic require is not supported');",
},
{
file: require.resolve("typescript"),
process(text) {
return text.replace(
/(?<=\n)(?<indentString>\s+)function tryGetNodePerformanceHooks\(\) {.*?\n\k<indentString>}(?=\n)/s,
"function tryGetNodePerformanceHooks() {}"
);
},
},

...Object.entries({
// `typescript/lib/typescript.js` expose extra global objects
Expand Down Expand Up @@ -262,6 +281,13 @@ const coreBundles = [
require.resolve("./shims/chalk.cjs"),
...replaceDiffPackageEntry("lib/diff/array.js"),
},
replaceText: [
{
file: path.join(PROJECT_ROOT, "src/main/parser.js"),
find: "return requireParser(opts.parser);",
replacement: "",
},
],
},
{
input: "bin/prettier.js",
Expand Down
24 changes: 19 additions & 5 deletions scripts/build/esbuild-plugins/replace-text.mjs
@@ -1,9 +1,5 @@
import fs from "node:fs/promises";

function replaceText(text, options) {
return text.replaceAll(options.find, options.replacement);
}

export default function esbuildPluginReplaceText({
filter = /./,
replacements,
Expand All @@ -12,6 +8,24 @@ export default function esbuildPluginReplaceText({
if (!replacement.file) {
throw new Error("'file' option is required.");
}

if (Object.prototype.hasOwnProperty.call(replacement, "process")) {
if (typeof replacement.process !== "function") {
throw new TypeError("'process' option should be a function.");
}

continue;
}

if (
!Object.prototype.hasOwnProperty.call(replacement, "find") ||
!Object.prototype.hasOwnProperty.call(replacement, "replacement")
) {
throw new Error("'find' and 'replacement' option is required.");
}

replacement.process = (text) =>
text.replaceAll(replacement.find, replacement.replacement);
}

return {
Expand All @@ -24,7 +38,7 @@ export default function esbuildPluginReplaceText({
if (replacement.file !== "*" && replacement.file !== file) {
continue;
}
text = replaceText(text, replacement);
text = replacement.process(text);
}

return { contents: text };
Expand Down
82 changes: 82 additions & 0 deletions scripts/tools/bundle-test/index.js
@@ -0,0 +1,82 @@
import { fileURLToPath } from "node:url";
import path from "node:path";
import crypto from "node:crypto";
import fs from "node:fs/promises";
import webpack from "webpack";
import { DIST_DIR } from "../../../scripts/utils/index.mjs";

function runWebpack(config) {
return new Promise((resolve, reject) => {
webpack(config, (error, stats) => {
if (error) {
reject(error);
return;
}

if (stats.hasErrors()) {
const { errors } = stats.toJson();
const error = new Error(errors[0].message);
error.errors = errors;
reject(error);
return;
}

resolve(stats);
});
});
}

const getRandomFileName = (prefix) =>
prefix + "-" + crypto.randomBytes(4).toString("hex").slice(0, 4) + ".js";

const TEMPORARY_DIRECTORY = fileURLToPath(new URL("./.tmp", import.meta.url));

/* `require` in `parser-typescript.js`, #12338 */
(async () => {
const esmFilesDirectory = path.join(DIST_DIR, "esm");

const files = [
(await fs.readdir(DIST_DIR))
.filter(
(name) =>
name.startsWith("parser-") ||
name === "standalone.js" ||
name === "doc.js"
)
.map((name) => ({ name, file: path.join(DIST_DIR, name) })),
(await fs.readdir(esmFilesDirectory)).map((name) => ({
displayName: `esm/${name}`,
name,
file: path.join(esmFilesDirectory, name),
})),
].flat();

for (const { displayName, name, file } of files) {
console.log(`${displayName || name}: `);

const stats = await runWebpack({
mode: "production",
entry: file,
output: {
path: TEMPORARY_DIRECTORY,
filename: getRandomFileName(name),
},
Copy link
Sponsor Member Author

@fisker fisker Mar 18, 2022

Choose a reason for hiding this comment

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

@alexander-akait Do you know how to prevent webpack write file to disk?

});
const result = stats.toJson();
const warnings = result.warnings.filter(
({ message }) =>
!(
message.startsWith("entrypoint size limit:") ||
message.startsWith("asset size limit:") ||
message.startsWith("webpack performance recommendations:")
)
);

if (warnings.length > 0) {
console.log(warnings);
throw new Error("Unexpected webpack warning.");
}

console.log(" Passed.");
}
})();
12 changes: 12 additions & 0 deletions scripts/tools/bundle-test/package.json
@@ -0,0 +1,12 @@
{
"name": "@prettier/bundle-test",
"version": "0.0.0",
"private": "true",
"type": "module",
"devDependencies": {
"webpack": "5.70.0"
},
"scripts": {
"test": "node ./index.js"
}
}