Skip to content

fix(npm): correct package_store_prefix_len for links in external-repo sub-packages - #2864

Merged
jbedard merged 3 commits into
aspect-build:mainfrom
oscar-meroton:fix-package-store-prefix-len
Sep 1, 2026
Merged

fix(npm): correct package_store_prefix_len for links in external-repo sub-packages#2864
jbedard merged 3 commits into
aspect-build:mainfrom
oscar-meroton:fix-package-store-prefix-len

Conversation

@oscar-meroton

Copy link
Copy Markdown
Contributor

When calculating the package store prefix truncation with external modules (i.e. a repo name is included in the path), an extra slash after the repo name was unaccounted for, resulting in incorrect symlinks (and subsequently ERR_MODULE_NOT_FOUND error during builds).

Below is an example of the logic used to calculate the truncation length:

# ../[com_github_example_repo+]/[example_package]/[node_modules/.aspect_rules_js/]foo@1.2.3/node_modules/bar
#    |        repo name       | |  package name | |         store prefix         |

path = "../com_github_example_repo+/example_package/node_modules/.aspect_rules_js/foo@1.2.3/node_modules/bar"
truncate_length = len(repo_name) + len(package_name) + len(store_prefix) + 3 # = 72
truncated_path = path[truncate_length:] # = "s/foo@1.2.3/node_modules/bar"

The extra s in the beginning of truncated_path should not be there and is solved by changing the + 3 to a + 4.

@jbedard

jbedard commented Jun 3, 2026

Copy link
Copy Markdown
Member

@oscar-meroton sorry I didn't see this till now. How can we reproduce this in a test?

@oscar-meroton

oscar-meroton commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

@jbedard Hey, sorry for the delay. I'll try to look into reproducing this in a test later this week.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@jbedard jbedard changed the title fix: Change 3 to 4 in npm_package_store prefix len fix(npm): correct package_store_prefix_len for links in external-repo sub-packages Sep 1, 2026
oscar-meroton and others added 3 commits September 1, 2026 14:52
When calculating the length of the package store prefix
for truncation, an extra slash after the repo name was
unaccounted for, resulting in incorrect symlinks.

Co-authored-by: Fredrik Medley <fredrik@meroton.com>
@jbedard
jbedard enabled auto-merge (squash) September 1, 2026 21:57
@jbedard
jbedard disabled auto-merge September 1, 2026 22:09
@jbedard
jbedard merged commit 048c545 into aspect-build:main Sep 1, 2026
165 of 166 checks passed
Sign up for free to 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.

3 participants