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

JS-380 Bundle bridge using esbuild #4901

Merged
merged 18 commits into from
Nov 14, 2024
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Maven
target/
bin/

lib/
node_modules/
Expand Down
110 changes: 110 additions & 0 deletions esbuild.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
import esbuild from 'esbuild';
import textReplace from 'esbuild-plugin-text-replace';
import { copy } from 'esbuild-plugin-copy';

await esbuild.build({
entryPoints: ['./server.mjs'],
outfile: './bin/server.cjs',
format: 'cjs',
bundle: true,
// we mark this file as external because it does not exist on EsLint any more and in any case
// the code never reaches this dynamic require as this is a fallback if 'eslint/use-at-your-own-risk'
// does not exist. we need to keep an eye on this in the future.
external: ['eslint/lib/util/glob-util'],
platform: 'node',
minify: true,
plugins: [
textReplace({
include: /server\.mjs$/,
pattern: [['new URL(import.meta.url)', '__filename']],
}),
textReplace({
include: /lib[\/\\]jsts[\/\\]src[\/\\]parsers[\/\\]ast\.js$/,
pattern: [['path.dirname(fileURLToPath(import.meta.url))', '__dirname']],
}),
// Simplify the loader function in babel. At the end it's just importing Babel parser
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think it would be worth to point a specific line in a github repo? Perhaps this way, if something changes, it would be a good starting point to investigate.

Also, it would be great, if we could assert that these replacements actually happened. If we could make the bundling fail if any of the replacements didn't occur....

// This matches the result of the TS compilation of the following lines
// https://github.com/babel/babel/blob/v7.25.1/eslint/babel-eslint-parser/src/parse.cts#L8-L12
textReplace({
include: /node_modules[\/\\]@babel[\/\\]eslint-parser[\/\\]lib[\/\\]parse\.cjs$/,
pattern: [
[/const babelParser = require.*}\)\)/gms, 'const babelParser = require("@babel/parser")'],
],
}),
// Remove dynamic import of espree on ESLint Rule tester. In any case, it's never used in the bundle
textReplace({
include: /node_modules[\/\\]eslint[\/\\]lib[\/\\]rule-tester[\/\\]rule-tester\.js$/,
pattern: [
// https://github.com/eslint/eslint/blob/v8.57.0/lib/rule-tester/rule-tester.js#L56
['const espreePath = require.resolve("espree");', ''],
// https://github.com/eslint/eslint/blob/v8.57.0/lib/rule-tester/rule-tester.js#L781
['config.parser = espreePath;', ''],
],
}),
// Dynamic import in module used by eslint-import-plugin. It always resolves to node resolver
textReplace({
include: /node_modules[\/\\]eslint-module-utils[\/\\]resolve\.js$/,
pattern: [
[
// https://github.com/import-js/eslint-plugin-import/blob/v2.11.0/utils/resolve.js#L157
'tryRequire(`eslint-import-resolver-${name}`, sourceFile)',
'require("eslint-import-resolver-node")',
],
],
}),
// the html extractor for stylelint calls a "loadSyntax" function in postcss-syntax/load-syntax.js
// That function has a dynamic require which always resolves to same dependencies given
// our stylelint options.
textReplace({
include: /node_modules[\/\\]postcss-html[\/\\]extract\.js$/,
pattern: [
[
//https://github.com/ota-meshi/postcss-html/blob/v0.36.0/extract.js#L108
'style.syntax = loadSyntax(opts, __dirname);',
`style.syntax = {
parse: require("postcss-html/template-parse"),
stringify: require("postcss/lib/stringify")
};
opts.syntax.config["css"] = {
stringify: require("postcss/lib/stringify"),
parse: require("postcss/lib/parse")
}`,
// ^^ modifying "opts.syntax.config" is a side effect done in postcss-syntax/get-syntax.js
],
],
}),
// The comparison by constructor name made by stylelint is not valid in the bundle because
// the Document object is named differently. We need to compare constructor object directly
textReplace({
include: /node_modules[\/\\]stylelint[\/\\]lib[\/\\]lintPostcssResult\.js$/,
pattern: [
[
// https://github.com/stylelint/stylelint/blob/15.10.0/lib/lintPostcssResult.js#L52
"postcssDoc && postcssDoc.constructor.name === 'Document' ? postcssDoc.nodes : [postcssDoc]",
"postcssDoc && postcssDoc.constructor === require('postcss-syntax/document') ? postcssDoc.nodes : [postcssDoc]",
],
],
}),
copy({
resolveFrom: 'cwd',
assets: [
// We need to copy all typescript declaration files for type-checking, as typescript will
// look for those in the filesystem
{
from: ['./node_modules/typescript/lib/*.d.ts'],
to: ['./bin/'],
},
// We copy run-node into the bundle, as it's used from the java side on Mac
{
from: ['./run-node'],
to: ['./bin/'],
},
// We copy the protofile as it needs to be accessible for the bundle
{
from: ['./packages/jsts/src/parsers/estree.proto'],
to: ['./bin/'],
},
],
}),
],
});
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ exports.rules = [
],
},
create(context) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For posteriority, the lines below were removed because typescript isn't installed and available

const ts = require('typescript');
console.log(`TS API in custom rule: TS version ${ts.version}`); // should print embedded typescript version

console.log('Rule context options: ', context.options);
return {
CallExpression(node) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ void test() {
)
)
.isEmpty();
assertThat(buildResult.getLogsLines(l -> l.contains("TS API in custom rule: TS version 5.6.2")))
assertThat(buildResult.getLogsLines(l -> l.contains("Rule context options:")))
.hasSize(2);
List<Issue> issues = findIssues("eslint-custom-rules:sqKey", orchestrator);
assertThat(issues).hasSize(2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ void start(Path dest) throws IOException {
port = findOpenPort();
String[] cmd = {
"node",
dest.resolve("package/bin/server.mjs").toString(),
dest.resolve("package/bin/server.cjs").toString(),
String.valueOf(port),
"127.0.0.1",
temp.toString(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import java.util.Set;
import java.util.stream.Stream;
import org.apache.commons.lang.StringUtils;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.junit.jupiter.api.parallel.Execution;
Expand Down Expand Up @@ -334,13 +333,13 @@ private static void instantiateTemplateRule(
Arrays.asList(
(
"name=\"" +
instantiationKey +
"\";key=\"" +
instantiationKey +
"\";markdown_description=\"" +
instantiationKey +
"\";" +
params
instantiationKey +
"\";key=\"" +
instantiationKey +
"\";markdown_description=\"" +
instantiationKey +
"\";" +
params
).split(";", 0)
)
)
Expand Down Expand Up @@ -372,8 +371,8 @@ private static void instantiateTemplateRule(
} else {
throw new IllegalStateException(
"Could not retrieve profile key : Template rule " +
ruleTemplateKey +
" has not been activated"
ruleTemplateKey +
" has not been activated"
);
}
}
Expand All @@ -389,4 +388,4 @@ static WsClient newAdminWsClient(Orchestrator orchestrator) {
.build()
);
}
}
}
Loading
Loading