Revert "feat!: Remove support for Node < v18 and webpack v4" - #887

Closed
andreiborza wants to merge 1 commit into
mainfrom
revert-886-timfish/feat/node-v18
Closed

Revert "feat!: Remove support for Node < v18 and webpack v4"#887
andreiborza wants to merge 1 commit into
mainfrom
revert-886-timfish/feat/node-v18

Conversation

@andreiborza

Copy link
Copy Markdown
Member

Reverts #886

We want to ship a v4 release first before getting this out

@github-actions

Copy link
Copy Markdown

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


Bug Fixes 🐛

  • (webpack) Deduplicate webpack deploys by chargome in #875

Internal Changes 🔧

  • Avoid direct usage of glob, extract into globFiles helper by andreiborza in #883
  • Migrate to oxfmt by timfish in #880
  • Build with Rolldown by timfish in #872
  • Remove unplugin by timfish in #876
  • Rollup/Vite no longer uses unplugin by timfish in #858
  • Esbuild no longer uses unplugin by timfish in #871
  • Webpack no longer uses unplugin by timfish in #870

🤖 This preview updates automatically when you update the PR.

Comment on lines +112 to +122
}/webpack4`,
},
}),
],
},
(err) => {
if (err) {
throw err;
}
}
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The e2e test utility create-cjs-bundles.ts unconditionally runs webpack4, which will fail on Node.js 18+ when the currently disabled e2e tests are re-enabled.
Severity: MEDIUM

Suggested Fix

Add a Node.js version check in packages/e2e-tests/utils/create-cjs-bundles.ts to skip the webpack4 build process when the major version is 18 or greater. This will align its behavior with similar scripts like create-cjs-bundles-for-react.ts.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/e2e-tests/utils/create-cjs-bundles.ts#L91-L122
Potential issue: The script at `packages/e2e-tests/utils/create-cjs-bundles.ts`
unconditionally executes a webpack4 build. This is problematic because webpack4 is
incompatible with Node.js versions 18 and higher, causing build failures. While the e2e
tests are currently disabled in the CI pipeline, this latent bug will manifest as soon
as they are re-enabled in an environment running a modern Node.js version. Other similar
utility scripts in the integration tests already contain guards to prevent this exact
issue by checking the Node.js version before attempting to run webpack4.

Did we get this right? 👍 / 👎 to inform future reviews.

@andreiborza

Copy link
Copy Markdown
MemberAuthor

Will just cherry-pick #875 onto a v4 branch...

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

export const testIfNodeMajorVersionIsLessThan18: jest.It = function () {
return testIf(NODE_MAJOR_VERSION < 18);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Webpack 4 tests silently never register or execute

High Severity

testIfNodeMajorVersionIsLessThan18 is defined as a wrapper function that calls testIf() and returns the result (test or test.skip), but it ignores the arguments passed to it (test name and callback). When invoked as testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => { ... }), the test name and callback are never forwarded to test/test.skip, so the test is silently never registered. Compare with describeNode18Plus, which correctly assigns the result of the condition directly. The fix is to evaluate testIf(NODE_MAJOR_VERSION < 18) eagerly and assign its result, rather than wrapping it in another function layer.

Fix in CursorFix in Web

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@andreiborza@chargome
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Revert "feat!: Remove support for Node < v18 and webpack v4" - #887

Closed
andreiborza wants to merge 1 commit into
mainfrom
revert-886-timfish/feat/node-v18
Closed

Revert "feat!: Remove support for Node < v18 and webpack v4"#887
andreiborza wants to merge 1 commit into
mainfrom
revert-886-timfish/feat/node-v18

Conversation

@andreiborza

Copy link
Copy Markdown
Member

Reverts #886

We want to ship a v4 release first before getting this out

@github-actions

Copy link
Copy Markdown

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


Bug Fixes 🐛

  • (webpack) Deduplicate webpack deploys by chargome in #875

Internal Changes 🔧

  • Avoid direct usage of glob, extract into globFiles helper by andreiborza in #883
  • Migrate to oxfmt by timfish in #880
  • Build with Rolldown by timfish in #872
  • Remove unplugin by timfish in #876
  • Rollup/Vite no longer uses unplugin by timfish in #858
  • Esbuild no longer uses unplugin by timfish in #871
  • Webpack no longer uses unplugin by timfish in #870

🤖 This preview updates automatically when you update the PR.

Comment on lines +112 to +122
}/webpack4`,
},
}),
],
},
(err) => {
if (err) {
throw err;
}
}
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The e2e test utility create-cjs-bundles.ts unconditionally runs webpack4, which will fail on Node.js 18+ when the currently disabled e2e tests are re-enabled.
Severity: MEDIUM

Suggested Fix

Add a Node.js version check in packages/e2e-tests/utils/create-cjs-bundles.ts to skip the webpack4 build process when the major version is 18 or greater. This will align its behavior with similar scripts like create-cjs-bundles-for-react.ts.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/e2e-tests/utils/create-cjs-bundles.ts#L91-L122
Potential issue: The script at `packages/e2e-tests/utils/create-cjs-bundles.ts`
unconditionally executes a webpack4 build. This is problematic because webpack4 is
incompatible with Node.js versions 18 and higher, causing build failures. While the e2e
tests are currently disabled in the CI pipeline, this latent bug will manifest as soon
as they are re-enabled in an environment running a modern Node.js version. Other similar
utility scripts in the integration tests already contain guards to prevent this exact
issue by checking the Node.js version before attempting to run webpack4.

Did we get this right? 👍 / 👎 to inform future reviews.

@andreiborza

Copy link
Copy Markdown
MemberAuthor

Will just cherry-pick #875 onto a v4 branch...

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

export const testIfNodeMajorVersionIsLessThan18: jest.It = function () {
return testIf(NODE_MAJOR_VERSION < 18);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Webpack 4 tests silently never register or execute

High Severity

testIfNodeMajorVersionIsLessThan18 is defined as a wrapper function that calls testIf() and returns the result (test or test.skip), but it ignores the arguments passed to it (test name and callback). When invoked as testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => { ... }), the test name and callback are never forwarded to test/test.skip, so the test is silently never registered. Compare with describeNode18Plus, which correctly assigns the result of the condition directly. The fix is to evaluate testIf(NODE_MAJOR_VERSION < 18) eagerly and assign its result, rather than wrapping it in another function layer.

Fix in CursorFix in Web

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@andreiborza@chargome
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Revert "feat!: Remove support for Node < v18 and webpack v4" - #887

Closed
andreiborza wants to merge 1 commit into
mainfrom
revert-886-timfish/feat/node-v18
Closed

Revert "feat!: Remove support for Node < v18 and webpack v4"#887
andreiborza wants to merge 1 commit into
mainfrom
revert-886-timfish/feat/node-v18

Conversation

@andreiborza

Copy link
Copy Markdown
Member

Reverts #886

We want to ship a v4 release first before getting this out

@github-actions

Copy link
Copy Markdown

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


Bug Fixes 🐛

  • (webpack) Deduplicate webpack deploys by chargome in #875

Internal Changes 🔧

  • Avoid direct usage of glob, extract into globFiles helper by andreiborza in #883
  • Migrate to oxfmt by timfish in #880
  • Build with Rolldown by timfish in #872
  • Remove unplugin by timfish in #876
  • Rollup/Vite no longer uses unplugin by timfish in #858
  • Esbuild no longer uses unplugin by timfish in #871
  • Webpack no longer uses unplugin by timfish in #870

🤖 This preview updates automatically when you update the PR.

Comment on lines +112 to +122
}/webpack4`,
},
}),
],
},
(err) => {
if (err) {
throw err;
}
}
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The e2e test utility create-cjs-bundles.ts unconditionally runs webpack4, which will fail on Node.js 18+ when the currently disabled e2e tests are re-enabled.
Severity: MEDIUM

Suggested Fix

Add a Node.js version check in packages/e2e-tests/utils/create-cjs-bundles.ts to skip the webpack4 build process when the major version is 18 or greater. This will align its behavior with similar scripts like create-cjs-bundles-for-react.ts.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/e2e-tests/utils/create-cjs-bundles.ts#L91-L122
Potential issue: The script at `packages/e2e-tests/utils/create-cjs-bundles.ts`
unconditionally executes a webpack4 build. This is problematic because webpack4 is
incompatible with Node.js versions 18 and higher, causing build failures. While the e2e
tests are currently disabled in the CI pipeline, this latent bug will manifest as soon
as they are re-enabled in an environment running a modern Node.js version. Other similar
utility scripts in the integration tests already contain guards to prevent this exact
issue by checking the Node.js version before attempting to run webpack4.

Did we get this right? 👍 / 👎 to inform future reviews.

@andreiborza

Copy link
Copy Markdown
MemberAuthor

Will just cherry-pick #875 onto a v4 branch...

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

export const testIfNodeMajorVersionIsLessThan18: jest.It = function () {
return testIf(NODE_MAJOR_VERSION < 18);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Webpack 4 tests silently never register or execute

High Severity

testIfNodeMajorVersionIsLessThan18 is defined as a wrapper function that calls testIf() and returns the result (test or test.skip), but it ignores the arguments passed to it (test name and callback). When invoked as testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => { ... }), the test name and callback are never forwarded to test/test.skip, so the test is silently never registered. Compare with describeNode18Plus, which correctly assigns the result of the condition directly. The fix is to evaluate testIf(NODE_MAJOR_VERSION < 18) eagerly and assign its result, rather than wrapping it in another function layer.

Fix in CursorFix in Web

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@andreiborza@chargome
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Revert "feat!: Remove support for Node < v18 and webpack v4" - #887

Closed
andreiborza wants to merge 1 commit into
mainfrom
revert-886-timfish/feat/node-v18
Closed

Revert "feat!: Remove support for Node < v18 and webpack v4"#887
andreiborza wants to merge 1 commit into
mainfrom
revert-886-timfish/feat/node-v18

Conversation

@andreiborza

Copy link
Copy Markdown
Member

Reverts #886

We want to ship a v4 release first before getting this out

@github-actions

Copy link
Copy Markdown

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


Bug Fixes 🐛

  • (webpack) Deduplicate webpack deploys by chargome in #875

Internal Changes 🔧

  • Avoid direct usage of glob, extract into globFiles helper by andreiborza in #883
  • Migrate to oxfmt by timfish in #880
  • Build with Rolldown by timfish in #872
  • Remove unplugin by timfish in #876
  • Rollup/Vite no longer uses unplugin by timfish in #858
  • Esbuild no longer uses unplugin by timfish in #871
  • Webpack no longer uses unplugin by timfish in #870

🤖 This preview updates automatically when you update the PR.

Comment on lines +112 to +122
}/webpack4`,
},
}),
],
},
(err) => {
if (err) {
throw err;
}
}
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The e2e test utility create-cjs-bundles.ts unconditionally runs webpack4, which will fail on Node.js 18+ when the currently disabled e2e tests are re-enabled.
Severity: MEDIUM

Suggested Fix

Add a Node.js version check in packages/e2e-tests/utils/create-cjs-bundles.ts to skip the webpack4 build process when the major version is 18 or greater. This will align its behavior with similar scripts like create-cjs-bundles-for-react.ts.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/e2e-tests/utils/create-cjs-bundles.ts#L91-L122
Potential issue: The script at `packages/e2e-tests/utils/create-cjs-bundles.ts`
unconditionally executes a webpack4 build. This is problematic because webpack4 is
incompatible with Node.js versions 18 and higher, causing build failures. While the e2e
tests are currently disabled in the CI pipeline, this latent bug will manifest as soon
as they are re-enabled in an environment running a modern Node.js version. Other similar
utility scripts in the integration tests already contain guards to prevent this exact
issue by checking the Node.js version before attempting to run webpack4.

Did we get this right? 👍 / 👎 to inform future reviews.

@andreiborza

Copy link
Copy Markdown
MemberAuthor

Will just cherry-pick #875 onto a v4 branch...

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

export const testIfNodeMajorVersionIsLessThan18: jest.It = function () {
return testIf(NODE_MAJOR_VERSION < 18);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Webpack 4 tests silently never register or execute

High Severity

testIfNodeMajorVersionIsLessThan18 is defined as a wrapper function that calls testIf() and returns the result (test or test.skip), but it ignores the arguments passed to it (test name and callback). When invoked as testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => { ... }), the test name and callback are never forwarded to test/test.skip, so the test is silently never registered. Compare with describeNode18Plus, which correctly assigns the result of the condition directly. The fix is to evaluate testIf(NODE_MAJOR_VERSION < 18) eagerly and assign its result, rather than wrapping it in another function layer.

Fix in CursorFix in Web

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@andreiborza@chargome
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Revert "feat!: Remove support for Node < v18 and webpack v4" - #887

Closed
andreiborza wants to merge 1 commit into
mainfrom
revert-886-timfish/feat/node-v18
Closed

Revert "feat!: Remove support for Node < v18 and webpack v4"#887
andreiborza wants to merge 1 commit into
mainfrom
revert-886-timfish/feat/node-v18

Conversation

@andreiborza

Copy link
Copy Markdown
Member

Reverts #886

We want to ship a v4 release first before getting this out

@github-actions

Copy link
Copy Markdown

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


Bug Fixes 🐛

  • (webpack) Deduplicate webpack deploys by chargome in #875

Internal Changes 🔧

  • Avoid direct usage of glob, extract into globFiles helper by andreiborza in #883
  • Migrate to oxfmt by timfish in #880
  • Build with Rolldown by timfish in #872
  • Remove unplugin by timfish in #876
  • Rollup/Vite no longer uses unplugin by timfish in #858
  • Esbuild no longer uses unplugin by timfish in #871
  • Webpack no longer uses unplugin by timfish in #870

🤖 This preview updates automatically when you update the PR.

Comment on lines +112 to +122
}/webpack4`,
},
}),
],
},
(err) => {
if (err) {
throw err;
}
}
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The e2e test utility create-cjs-bundles.ts unconditionally runs webpack4, which will fail on Node.js 18+ when the currently disabled e2e tests are re-enabled.
Severity: MEDIUM

Suggested Fix

Add a Node.js version check in packages/e2e-tests/utils/create-cjs-bundles.ts to skip the webpack4 build process when the major version is 18 or greater. This will align its behavior with similar scripts like create-cjs-bundles-for-react.ts.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/e2e-tests/utils/create-cjs-bundles.ts#L91-L122
Potential issue: The script at `packages/e2e-tests/utils/create-cjs-bundles.ts`
unconditionally executes a webpack4 build. This is problematic because webpack4 is
incompatible with Node.js versions 18 and higher, causing build failures. While the e2e
tests are currently disabled in the CI pipeline, this latent bug will manifest as soon
as they are re-enabled in an environment running a modern Node.js version. Other similar
utility scripts in the integration tests already contain guards to prevent this exact
issue by checking the Node.js version before attempting to run webpack4.

Did we get this right? 👍 / 👎 to inform future reviews.

@andreiborza

Copy link
Copy Markdown
MemberAuthor

Will just cherry-pick #875 onto a v4 branch...

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

export const testIfNodeMajorVersionIsLessThan18: jest.It = function () {
return testIf(NODE_MAJOR_VERSION < 18);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Webpack 4 tests silently never register or execute

High Severity

testIfNodeMajorVersionIsLessThan18 is defined as a wrapper function that calls testIf() and returns the result (test or test.skip), but it ignores the arguments passed to it (test name and callback). When invoked as testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => { ... }), the test name and callback are never forwarded to test/test.skip, so the test is silently never registered. Compare with describeNode18Plus, which correctly assigns the result of the condition directly. The fix is to evaluate testIf(NODE_MAJOR_VERSION < 18) eagerly and assign its result, rather than wrapping it in another function layer.

Fix in CursorFix in Web

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@andreiborza@chargome
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Revert "feat!: Remove support for Node < v18 and webpack v4" - #887

Closed
andreiborza wants to merge 1 commit into
mainfrom
revert-886-timfish/feat/node-v18
Closed

Revert "feat!: Remove support for Node < v18 and webpack v4"#887
andreiborza wants to merge 1 commit into
mainfrom
revert-886-timfish/feat/node-v18

Conversation

@andreiborza

Copy link
Copy Markdown
Member

Reverts #886

We want to ship a v4 release first before getting this out

@github-actions

Copy link
Copy Markdown

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


Bug Fixes 🐛

  • (webpack) Deduplicate webpack deploys by chargome in #875

Internal Changes 🔧

  • Avoid direct usage of glob, extract into globFiles helper by andreiborza in #883
  • Migrate to oxfmt by timfish in #880
  • Build with Rolldown by timfish in #872
  • Remove unplugin by timfish in #876
  • Rollup/Vite no longer uses unplugin by timfish in #858
  • Esbuild no longer uses unplugin by timfish in #871
  • Webpack no longer uses unplugin by timfish in #870

🤖 This preview updates automatically when you update the PR.

Comment on lines +112 to +122
}/webpack4`,
},
}),
],
},
(err) => {
if (err) {
throw err;
}
}
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The e2e test utility create-cjs-bundles.ts unconditionally runs webpack4, which will fail on Node.js 18+ when the currently disabled e2e tests are re-enabled.
Severity: MEDIUM

Suggested Fix

Add a Node.js version check in packages/e2e-tests/utils/create-cjs-bundles.ts to skip the webpack4 build process when the major version is 18 or greater. This will align its behavior with similar scripts like create-cjs-bundles-for-react.ts.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/e2e-tests/utils/create-cjs-bundles.ts#L91-L122
Potential issue: The script at `packages/e2e-tests/utils/create-cjs-bundles.ts`
unconditionally executes a webpack4 build. This is problematic because webpack4 is
incompatible with Node.js versions 18 and higher, causing build failures. While the e2e
tests are currently disabled in the CI pipeline, this latent bug will manifest as soon
as they are re-enabled in an environment running a modern Node.js version. Other similar
utility scripts in the integration tests already contain guards to prevent this exact
issue by checking the Node.js version before attempting to run webpack4.

Did we get this right? 👍 / 👎 to inform future reviews.

@andreiborza

Copy link
Copy Markdown
MemberAuthor

Will just cherry-pick #875 onto a v4 branch...

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

export const testIfNodeMajorVersionIsLessThan18: jest.It = function () {
return testIf(NODE_MAJOR_VERSION < 18);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Webpack 4 tests silently never register or execute

High Severity

testIfNodeMajorVersionIsLessThan18 is defined as a wrapper function that calls testIf() and returns the result (test or test.skip), but it ignores the arguments passed to it (test name and callback). When invoked as testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => { ... }), the test name and callback are never forwarded to test/test.skip, so the test is silently never registered. Compare with describeNode18Plus, which correctly assigns the result of the condition directly. The fix is to evaluate testIf(NODE_MAJOR_VERSION < 18) eagerly and assign its result, rather than wrapping it in another function layer.

Fix in CursorFix in Web

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@andreiborza@chargome
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Revert "feat!: Remove support for Node < v18 and webpack v4" - #887

Closed
andreiborza wants to merge 1 commit into
mainfrom
revert-886-timfish/feat/node-v18
Closed

Revert "feat!: Remove support for Node < v18 and webpack v4"#887
andreiborza wants to merge 1 commit into
mainfrom
revert-886-timfish/feat/node-v18

Conversation

@andreiborza

Copy link
Copy Markdown
Member

Reverts #886

We want to ship a v4 release first before getting this out

@github-actions

Copy link
Copy Markdown

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


Bug Fixes 🐛

  • (webpack) Deduplicate webpack deploys by chargome in #875

Internal Changes 🔧

  • Avoid direct usage of glob, extract into globFiles helper by andreiborza in #883
  • Migrate to oxfmt by timfish in #880
  • Build with Rolldown by timfish in #872
  • Remove unplugin by timfish in #876
  • Rollup/Vite no longer uses unplugin by timfish in #858
  • Esbuild no longer uses unplugin by timfish in #871
  • Webpack no longer uses unplugin by timfish in #870

🤖 This preview updates automatically when you update the PR.

Comment on lines +112 to +122
}/webpack4`,
},
}),
],
},
(err) => {
if (err) {
throw err;
}
}
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The e2e test utility create-cjs-bundles.ts unconditionally runs webpack4, which will fail on Node.js 18+ when the currently disabled e2e tests are re-enabled.
Severity: MEDIUM

Suggested Fix

Add a Node.js version check in packages/e2e-tests/utils/create-cjs-bundles.ts to skip the webpack4 build process when the major version is 18 or greater. This will align its behavior with similar scripts like create-cjs-bundles-for-react.ts.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/e2e-tests/utils/create-cjs-bundles.ts#L91-L122
Potential issue: The script at `packages/e2e-tests/utils/create-cjs-bundles.ts`
unconditionally executes a webpack4 build. This is problematic because webpack4 is
incompatible with Node.js versions 18 and higher, causing build failures. While the e2e
tests are currently disabled in the CI pipeline, this latent bug will manifest as soon
as they are re-enabled in an environment running a modern Node.js version. Other similar
utility scripts in the integration tests already contain guards to prevent this exact
issue by checking the Node.js version before attempting to run webpack4.

Did we get this right? 👍 / 👎 to inform future reviews.

@andreiborza

Copy link
Copy Markdown
MemberAuthor

Will just cherry-pick #875 onto a v4 branch...

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

export const testIfNodeMajorVersionIsLessThan18: jest.It = function () {
return testIf(NODE_MAJOR_VERSION < 18);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Webpack 4 tests silently never register or execute

High Severity

testIfNodeMajorVersionIsLessThan18 is defined as a wrapper function that calls testIf() and returns the result (test or test.skip), but it ignores the arguments passed to it (test name and callback). When invoked as testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => { ... }), the test name and callback are never forwarded to test/test.skip, so the test is silently never registered. Compare with describeNode18Plus, which correctly assigns the result of the condition directly. The fix is to evaluate testIf(NODE_MAJOR_VERSION < 18) eagerly and assign its result, rather than wrapping it in another function layer.

Fix in CursorFix in Web

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@andreiborza@chargome
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Revert "feat!: Remove support for Node < v18 and webpack v4" - #887

Closed
andreiborza wants to merge 1 commit into
mainfrom
revert-886-timfish/feat/node-v18
Closed

Revert "feat!: Remove support for Node < v18 and webpack v4"#887
andreiborza wants to merge 1 commit into
mainfrom
revert-886-timfish/feat/node-v18

Conversation

@andreiborza

Copy link
Copy Markdown
Member

Reverts #886

We want to ship a v4 release first before getting this out

@github-actions

Copy link
Copy Markdown

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


Bug Fixes 🐛

  • (webpack) Deduplicate webpack deploys by chargome in #875

Internal Changes 🔧

  • Avoid direct usage of glob, extract into globFiles helper by andreiborza in #883
  • Migrate to oxfmt by timfish in #880
  • Build with Rolldown by timfish in #872
  • Remove unplugin by timfish in #876
  • Rollup/Vite no longer uses unplugin by timfish in #858
  • Esbuild no longer uses unplugin by timfish in #871
  • Webpack no longer uses unplugin by timfish in #870

🤖 This preview updates automatically when you update the PR.

Comment on lines +112 to +122
}/webpack4`,
},
}),
],
},
(err) => {
if (err) {
throw err;
}
}
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The e2e test utility create-cjs-bundles.ts unconditionally runs webpack4, which will fail on Node.js 18+ when the currently disabled e2e tests are re-enabled.
Severity: MEDIUM

Suggested Fix

Add a Node.js version check in packages/e2e-tests/utils/create-cjs-bundles.ts to skip the webpack4 build process when the major version is 18 or greater. This will align its behavior with similar scripts like create-cjs-bundles-for-react.ts.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/e2e-tests/utils/create-cjs-bundles.ts#L91-L122
Potential issue: The script at `packages/e2e-tests/utils/create-cjs-bundles.ts`
unconditionally executes a webpack4 build. This is problematic because webpack4 is
incompatible with Node.js versions 18 and higher, causing build failures. While the e2e
tests are currently disabled in the CI pipeline, this latent bug will manifest as soon
as they are re-enabled in an environment running a modern Node.js version. Other similar
utility scripts in the integration tests already contain guards to prevent this exact
issue by checking the Node.js version before attempting to run webpack4.

Did we get this right? 👍 / 👎 to inform future reviews.

@andreiborza

Copy link
Copy Markdown
MemberAuthor

Will just cherry-pick #875 onto a v4 branch...

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

export const testIfNodeMajorVersionIsLessThan18: jest.It = function () {
return testIf(NODE_MAJOR_VERSION < 18);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Webpack 4 tests silently never register or execute

High Severity

testIfNodeMajorVersionIsLessThan18 is defined as a wrapper function that calls testIf() and returns the result (test or test.skip), but it ignores the arguments passed to it (test name and callback). When invoked as testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => { ... }), the test name and callback are never forwarded to test/test.skip, so the test is silently never registered. Compare with describeNode18Plus, which correctly assigns the result of the condition directly. The fix is to evaluate testIf(NODE_MAJOR_VERSION < 18) eagerly and assign its result, rather than wrapping it in another function layer.

Fix in CursorFix in Web

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@andreiborza@chargome