Skip to content

Commit 232942e

Browse files
authored
poc: preserve comments in esbuild (#1565)
* poc: preserve comments in esbuild * comment * more test cases * more test cases
1 parent 954f504 commit 232942e

File tree

4 files changed

+284
-2
lines changed

4 files changed

+284
-2
lines changed

injected/scripts/utils/build.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { readFileSync } from 'fs';
33
import { cwd } from '../../../scripts/script-utils.js';
44
import { join } from 'path';
55
import * as esbuild from 'esbuild';
6+
import { commentPlugin } from './comment-plugin.js';
67
const ROOT = join(cwd(import.meta.url), '..', '..');
78
const DEBUG = false;
89

@@ -38,6 +39,7 @@ export async function bundle(params) {
3839
format: 'iife',
3940
bundle: true,
4041
metafile: true,
42+
legalComments: 'inline',
4143
globalName: name,
4244
loader: {
4345
'.css': 'text',
@@ -48,7 +50,7 @@ export async function bundle(params) {
4850
'import.meta.injectName': JSON.stringify(platform),
4951
'import.meta.trackerLookup': trackerLookup,
5052
},
51-
plugins: [loadFeaturesPlugin],
53+
plugins: [loadFeaturesPlugin, commentPlugin()],
5254
banner: {
5355
js: prefixMessage,
5456
},
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import { promises } from 'node:fs';
2+
3+
/**
4+
* @returns {import("esbuild").Plugin}
5+
*/
6+
export function commentPlugin() {
7+
const PLUGIN_ID = 'comment-override';
8+
9+
/** @type {import("esbuild").Plugin} */
10+
const plugin = {
11+
name: PLUGIN_ID,
12+
setup(build) {
13+
build.onLoad({ filter: /.*/ }, async (args) => {
14+
if (!args.path.includes('node_modules')) return undefined;
15+
const text = await promises.readFile(args.path, 'utf8');
16+
return {
17+
contents: convertToLegalComments(text.toString()),
18+
loader: 'js',
19+
};
20+
});
21+
},
22+
};
23+
return plugin;
24+
}
25+
26+
/**
27+
* Detect the start of a particular comment and change the
28+
* lines to have the prefix `//!` - this allows esbuild to keep it
29+
*
30+
* When a line is matched, continue to match further lines until a non-comment is seen.
31+
*
32+
* @param {string} source
33+
*/
34+
export function convertToLegalComments(source) {
35+
// Process block comments - find all block comments
36+
const blockComments = source.match(/\/\*[\s\S]*?\*\//g) || [];
37+
38+
// Selectively replace only block comments that contain "copyright"
39+
let modifiedSource = source;
40+
for (const comment of blockComments) {
41+
if (/copyright/i.test(comment)) {
42+
// Replace only the block comments with copyright
43+
modifiedSource = modifiedSource.replace(comment, comment.replace(/\/\*/, '/*!'));
44+
}
45+
}
46+
47+
// Process line comments
48+
const lines = modifiedSource.split('\n');
49+
const result = [];
50+
let inCommentBlock = false;
51+
52+
for (let i = 0; i < lines.length; i++) {
53+
const line = lines[i];
54+
55+
// Check if the line contains a block comment - this breaks any line comment sequence
56+
if (line.includes('/*') || line.includes('*/')) {
57+
inCommentBlock = false;
58+
result.push(line);
59+
}
60+
// Check if this line starts a line comment block with "copyright"
61+
else if (!inCommentBlock && /^\s*\/\/(?=.*copyright.*$)/i.test(line)) {
62+
// Start of a copyright comment - mark it and convert
63+
inCommentBlock = true;
64+
result.push(line.replace(/^\s*\/\//, (match) => match.replace('//', '//!')));
65+
}
66+
// Check if we're continuing a line comment block
67+
else if (inCommentBlock && /^\s*\/\//.test(line)) {
68+
// Continue the comment block - convert the prefix
69+
result.push(line.replace(/^\s*\/\//, (match) => match.replace('//', '//!')));
70+
}
71+
// Check if we're exiting a comment block
72+
else {
73+
// Not a comment line or doesn't match our criteria, end the block
74+
inCommentBlock = false;
75+
result.push(line);
76+
}
77+
}
78+
79+
return result.join('\n');
80+
}

injected/unit-test/comment-plugin.js

Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
1+
import { convertToLegalComments } from '../scripts/utils/comment-plugin.js';
2+
3+
describe('convertToLegalComments', () => {
4+
it('should convert single line comments with copyright', () => {
5+
const input = `// This is a copyright notice
6+
const foo = 'bar';`;
7+
8+
const expected = `//! This is a copyright notice
9+
const foo = 'bar';`;
10+
11+
expect(convertToLegalComments(input)).toEqual(expected);
12+
});
13+
14+
it('should convert multiple consecutive line comments following a copyright line', () => {
15+
const input = `// This is a copyright notice
16+
// This is a second line
17+
// This is a third line
18+
const foo = 'bar';`;
19+
20+
const expected = `//! This is a copyright notice
21+
//! This is a second line
22+
//! This is a third line
23+
const foo = 'bar';`;
24+
25+
expect(convertToLegalComments(input)).toEqual(expected);
26+
});
27+
28+
it('should stop converting after a non-comment line is encountered', () => {
29+
const input = `// This is a copyright notice
30+
// This is a second line
31+
const foo = 'bar';
32+
// This is a regular comment that should not be converted`;
33+
34+
const expected = `//! This is a copyright notice
35+
//! This is a second line
36+
const foo = 'bar';
37+
// This is a regular comment that should not be converted`;
38+
39+
expect(convertToLegalComments(input)).toEqual(expected);
40+
});
41+
42+
it('should handle multiple separate comment blocks', () => {
43+
const input = `// This is a regular comment
44+
const a = 1;
45+
46+
// This has copyright info
47+
// And continues here
48+
const b = 2;
49+
50+
// Another copyright notice
51+
// With more details
52+
// And even more info
53+
const c = 3;`;
54+
55+
const expected = `// This is a regular comment
56+
const a = 1;
57+
58+
//! This has copyright info
59+
//! And continues here
60+
const b = 2;
61+
62+
//! Another copyright notice
63+
//! With more details
64+
//! And even more info
65+
const c = 3;`;
66+
67+
expect(convertToLegalComments(input)).toEqual(expected);
68+
});
69+
70+
it('should handle indented comments', () => {
71+
const input = `function test() {
72+
// This has copyright info
73+
// This is indented
74+
return true;
75+
}`;
76+
77+
const expected = `function test() {
78+
//! This has copyright info
79+
//! This is indented
80+
return true;
81+
}`;
82+
83+
expect(convertToLegalComments(input)).toEqual(expected);
84+
});
85+
86+
it('should handle block comments with copyright', () => {
87+
const input = `/* This is a copyright block comment */
88+
const foo = 'bar';
89+
90+
/* This is a regular
91+
multiline comment */`;
92+
93+
const expected = `/*! This is a copyright block comment */
94+
const foo = 'bar';
95+
96+
/* This is a regular
97+
multiline comment */`;
98+
99+
expect(convertToLegalComments(input)).toEqual(expected);
100+
});
101+
102+
it('should handle mixed comment types', () => {
103+
const input = `// This has copyright info
104+
// This continues
105+
/* This is a regular block comment */
106+
const foo = 'bar';
107+
108+
/* This is a copyright block comment */
109+
// This is a regular comment after a block`;
110+
111+
const expected = `//! This has copyright info
112+
//! This continues
113+
/* This is a regular block comment */
114+
const foo = 'bar';
115+
116+
/*! This is a copyright block comment */
117+
// This is a regular comment after a block`;
118+
119+
expect(convertToLegalComments(input)).toEqual(expected);
120+
});
121+
122+
it('should handle block comments breaking line comment sequences', () => {
123+
const input = `// This has copyright info
124+
// This line should be converted
125+
/* This block comment breaks the sequence */
126+
// This line should NOT be converted
127+
// Even though it follows another comment`;
128+
129+
const expected = `//! This has copyright info
130+
//! This line should be converted
131+
/* This block comment breaks the sequence */
132+
// This line should NOT be converted
133+
// Even though it follows another comment`;
134+
135+
expect(convertToLegalComments(input)).toEqual(expected);
136+
});
137+
138+
it('should handle case insensitivity for "copyright"', () => {
139+
const input = `// This has COPYRIGHT info
140+
// This continues
141+
const foo = 'bar';
142+
143+
// This has Copyright mixed case
144+
// More comments
145+
const baz = 'qux';`;
146+
147+
const expected = `//! This has COPYRIGHT info
148+
//! This continues
149+
const foo = 'bar';
150+
151+
//! This has Copyright mixed case
152+
//! More comments
153+
const baz = 'qux';`;
154+
155+
expect(convertToLegalComments(input)).toEqual(expected);
156+
});
157+
158+
it('should not convert comments without copyright', () => {
159+
const input = `// This is a regular comment
160+
// Another regular comment
161+
const foo = 'bar';`;
162+
163+
const expected = `// This is a regular comment
164+
// Another regular comment
165+
const foo = 'bar';`;
166+
167+
expect(convertToLegalComments(input)).toEqual(expected);
168+
});
169+
170+
it('should handle code with no comments', () => {
171+
const input = `const foo = 'bar';
172+
function test() {
173+
return true;
174+
}
175+
const obj = { key: 'value' };`;
176+
177+
const expected = input; // Should remain unchanged
178+
179+
expect(convertToLegalComments(input)).toEqual(expected);
180+
});
181+
182+
it('should treat empty lines as non-comment lines that break the sequence', () => {
183+
const input = `// This has copyright info
184+
// This continues
185+
186+
// These comments should NOT be converted
187+
// Because empty line breaks the sequence
188+
const foo = 'bar';`;
189+
190+
const expected = `//! This has copyright info
191+
//! This continues
192+
193+
// These comments should NOT be converted
194+
// Because empty line breaks the sequence
195+
const foo = 'bar';`;
196+
197+
expect(convertToLegalComments(input)).toEqual(expected);
198+
});
199+
});

injected/unit-test/verify-artifacts.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ const checks = {
2323
tests: [
2424
{ kind: 'maxFileSize', value: CSS_OUTPUT_SIZE },
2525
{ kind: 'containsString', text: '$TRACKER_LOOKUP$', includes: true },
26+
{ kind: 'containsString', text: 'Copyright (C) 2010 by Johannes Baagøe <[email protected]>', includes: true },
2627
],
2728
},
2829
firefox: {
@@ -34,7 +35,7 @@ const checks = {
3435
},
3536
integration: {
3637
file: join(BUILD, 'integration/contentScope.js'),
37-
tests: [{ kind: 'containsString', text: 'trackerLookup: ', includes: true }],
38+
tests: [{ kind: 'containsString', text: 'init_define_import_meta_trackerLookup = ', includes: true }],
3839
},
3940
windows: {
4041
file: join(BUILD, 'windows/contentScope.js'),

0 commit comments

Comments
 (0)