fix(framework): avoid excluding files with test in filename#15345
fix(framework): avoid excluding files with test in filename#15345Sonu99kr wants to merge 4 commits into
Conversation
🦋 Changeset detectedLatest commit: c71041f The changes in this PR will be included in the next version bump. This PR includes changesets to release 78 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@Sonu99kr is attempting to deploy a commit to the medusajs Team on Vercel. A member of the Team first needs to authorize it. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
Reviewed by Cursor Bugbot for commit da5bdb5. Configure here.
|
Thank you for your contribution, @Sonu99kr! After reviewing this PR, we need a few things addressed before we can move forward: Required changes:
Potential Bugs:
const segments = relativeFileName.split(path.sep)
return \!chunksToIgnore.some((chunk) => segments.includes(chunk))
The single-segment chunks ( A fix that handles both cases: const normalizedFile = relativeFileName.split(path.sep).join("/")
return \!chunksToIgnore.some((chunk) => {
const normalizedChunk = chunk.split(/[\/]/).join("/")
if (\!normalizedChunk.includes("/")) {
// single-segment: match against path segments only
return normalizedFile.split("/").includes(normalizedChunk)
}
// multi-segment: match as a path prefix
return normalizedFile.startsWith(normalizedChunk + "/")
})Once those changes are in place, we'll be happy to move this forward. Thanks again! Triggered by: new PR opened |
|
Thank you for your contribution, @Sonu99kr! The The following items from the prior review are still outstanding: Required changes:
Once those are addressed, we'll be happy to move this forward. Thanks again! Triggered by: new commit pushed |
|
Thank you for the continued progress, @Sonu99kr! The The following items from the prior review are still outstanding, plus one new issue with the changeset: Required changes:
Once those are addressed, we'll be happy to move this forward. Thanks! Triggered by: new commit pushed |
|
Thank you for the continued progress, @Sonu99kr! Good work adding the linked issue — that resolves one of the outstanding items. The following items from the prior review are still unresolved: Required changes:
Once those are addressed, we'll be happy to move this forward. Thanks! Triggered by: PR description updated |
|
You have used all of your free Bugbot PR reviews. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
|
Thank you for your continued effort and patience on this, @Sonu99kr! All items from the previous reviews have been resolved: ✅ A team member will do a final review before this is merged. We appreciate your patience! Notes: The test in Triggered by: new commit pushed |
1 participant
Fixes #15341
Replaced the substring match with path-segment based filtering to avoid incorrectly excluding files whose filenames contain
test, while preserving exclusions for ignored directories such assrc/testandsrc/admin.Summary
What — What changes are introduced in this PR?
This PR fixes an issue in the framework build compiler where files were incorrectly excluded from the build output if their path contained the substring
test, even whentestappeared only in the filename.Previously, the compiler filtered files using:
which caused valid files such as:
reset-test-vendor-password.tsseed-test-accounts.tsto be silently skipped during
medusa build.The filtering logic now matches against path segments instead of arbitrary substrings.
Why — Why are these changes relevant or necessary?
The current behavior unintentionally excludes user-authored scripts and fixtures whose filenames contain
test, even when they are not located inside ignored directories like:src/testintegration-testsunit-testsThis can silently prevent important scripts from appearing in the final build output, making the issue difficult to diagnose.
The fix preserves the intended behavior of ignoring test directories while allowing valid filenames containing
testto compile correctly.How — How have these changes been implemented?
The compiler filtering logic in:
was updated from substring matching:
to path-segment matching:
This ensures only actual path segments such as
testorintegration-testsare excluded.Testing — How have these changes been tested, or how can the reviewer test the feature?
Verified locally using a Medusa test application linked against the modified local framework package.
Reproduction Test
Created:
and ran:
Verified that:
is now correctly generated.
Regression Verification
Created:
and verified that:
is still excluded from the build output.
Examples
Additional Context
This issue was caused by broad substring matching against the full relative path, which unintentionally matched filenames containing
test.The updated implementation limits exclusion behavior to actual path segments, preserving the intended semantics of ignored directories.
Note
Medium Risk
Touches the build compiler’s file inclusion/exclusion logic; mistakes could cause unintended files to be skipped or included in build output across platforms.
Overview
Fixes
Compiler.#emitBuildOutputfiltering so ignored chunks (e.g.,test,integration-tests) match path segments/prefixes rather than arbitrary substrings in the full relative path, preventing files likereset-test-*.tsfrom being incorrectly excluded.Adds path normalization to
/and differentiates single-segment ignores (segment match) vs multi-segment ignores (directory-prefix match). Includes a patch changeset for@medusajs/framework.Reviewed by Cursor Bugbot for commit ead4adf. Bugbot is set up for automated code reviews on this repo. Configure here.