Skip to content

chore: rolldown dual build #1126

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

chore: rolldown dual build #1126

wants to merge 6 commits into from

Conversation

BioPhoton
Copy link
Collaborator

No description provided.

@github-actions github-actions bot added 📖 Project documentation improvements or additions to the project documentation 🔬 testing writing tests 🛠️ tooling labels Oct 5, 2025
@nx-cloud Nx Cloud
Copy link

nx-cloud bot commented Oct 5, 2025 •
edited
Loading

View your CI Pipeline Execution ↗ for commit 87adb1c

Command Status Duration Result
nx code-pushup --nx-bail -- print-config --outp... ❌ Failed 3m 23s View ↗

☁️ Nx Cloud last updated this comment at 2025-10-05 20:03:46 UTC

@pkg-pr-new pkg.pr.new
Copy link

pkg-pr-new bot commented Oct 5, 2025 •
edited
Loading

Open in StackBlitz

@code-pushup/ci

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/ci@1126

@code-pushup/cli

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/cli@1126

@code-pushup/core

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/core@1126

@code-pushup/create-cli

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/create-cli@1126

@code-pushup/models

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/models@1126

@code-pushup/nx-plugin

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/nx-plugin@1126

@code-pushup/coverage-plugin

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/coverage-plugin@1126

@code-pushup/eslint-plugin

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/eslint-plugin@1126

@code-pushup/jsdocs-plugin

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/jsdocs-plugin@1126

@code-pushup/js-packages-plugin

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/js-packages-plugin@1126

@code-pushup/lighthouse-plugin

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/lighthouse-plugin@1126

@code-pushup/typescript-plugin

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/typescript-plugin@1126

@code-pushup/utils

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/utils@1126

@code-pushup/models-transformers

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/models-transformers@1126

commit: 87adb1c

github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 5, 2025
View reviewed changes
rolldown.base.ts
/(['"`])((?:\.\.\/)+)(package\.json)\1/g,
(match, quote, dots, file) => {
// Remove one ../ from the path
const newDots = dots.replace(/\.\.\//, '');

Check failure

Code scanning / CodeQL

Incomplete string escaping or encoding High

This replaces only the first occurrence of /..//.

Copilot Autofix

AI 3 days ago

To fix the issue, we should ensure that the code reliably removes only the first occurrence of "../" from the matched group, but if the intention is to remove all occurrences (which the regex allows for), then it should use the correct replace logic. If the intention is to only remove one, replace with no g flag is sufficient, but the token may have more than one "../" so the code is potentially misleading. If the goal is to remove all, use /\.\.\//g. If only one should be removed, consider a clearer approach such as slicing or a specific replace for the first occurrence, making the operation and intent explicit. In this case, since the comments say "remove one", the code as written is correct but subject to confusion in future maintenance.

To fully address the CodeQL warning and ensure clear, predictable behavior, update the code to use a regular expression with the global flag if the goal is to remove all occurrences; otherwise, document and clarify why only one is removed. For safety and clarity, better to use /\.\.\//g unless there's a strict reason to only remove one.

No new imports are required for this change.

Suggested changeset 1
rolldown.base.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/rolldown.base.ts b/rolldown.base.ts
--- a/rolldown.base.ts
+++ b/rolldown.base.ts
@@ -259,7 +259,7 @@
               /(['"`])((?:\.\.\/)+)(package\.json)\1/g,
               (match, quote, dots, file) => {
                 // Remove one ../ from the path
-                const newDots = dots.replace(/\.\.\//, '');
+                const newDots = dots.replace(/\.\.\//g, '');
                 return `${quote}${newDots}${file}${quote}`;
               },
             );
EOF
@@ -259,7 +259,7 @@
/(['"`])((?:\.\.\/)+)(package\.json)\1/g,
(match, quote, dots, file) => {
// Remove one ../ from the path
const newDots = dots.replace(/\.\.\//, '');
const newDots = dots.replace(/\.\.\//g, '');
return `${quote}${newDots}${file}${quote}`;
},
);
Copilot is powered by AI and may make mistakes. Always verify output.
rolldown.base.ts
/(['"`])((?:\.\.\/)+)(package\.json)\1/g,
(match, quote, dots, file) => {
// Remove one ../ from the path
const newDots = dots.replace(/\.\.\//, '');

Check failure

Code scanning / CodeQL

Incomplete multi-character sanitization High

This string may still contain
../
Loading
, which may cause a path injection vulnerability.

Copilot Autofix

AI 3 days ago

To fix this problem, we need to ensure that all occurrences of '../' in the dots string are removed, not just the first one. This can be done by replacing the .replace(/\.\.\//, '') call with .replace(/\.\.\//g, ''), where the g flag ensures a global replacement within the string. This change will not affect other code paths or functionality — it simply expands the match to all instances, properly cleaning the path prefix as the plugin intended.

Only modify the relevant line in the plugin's transform function. No new imports, helper methods, or dependencies are required.

Suggested changeset 1
rolldown.base.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/rolldown.base.ts b/rolldown.base.ts
--- a/rolldown.base.ts
+++ b/rolldown.base.ts
@@ -259,7 +259,7 @@
               /(['"`])((?:\.\.\/)+)(package\.json)\1/g,
               (match, quote, dots, file) => {
                 // Remove one ../ from the path
-                const newDots = dots.replace(/\.\.\//, '');
+                const newDots = dots.replace(/\.\.\//g, '');
                 return `${quote}${newDots}${file}${quote}`;
               },
             );
EOF
@@ -259,7 +259,7 @@
/(['"`])((?:\.\.\/)+)(package\.json)\1/g,
(match, quote, dots, file) => {
// Remove one ../ from the path
const newDots = dots.replace(/\.\.\//, '');
const newDots = dots.replace(/\.\.\//g, '');
return `${quote}${newDots}${file}${quote}`;
},
);
Copilot is powered by AI and may make mistakes. Always verify output.
@BioPhoton BioPhoton changed the title chore: rolldown build chore: rolldown dual build Oct 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matejchalk matejchalk Awaiting requested review from matejchalk matejchalk will be requested when the pull request is marked ready for review matejchalk is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@BioPhoton BioPhoton

Labels
📖 Project documentation improvements or additions to the project documentation 🔬 testing writing tests 🛠️ tooling
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@BioPhoton