Skip to content
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.

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

fix(reflection): ensure complete stripping of relative paths with multiple leading slashes #4844

Merged

Conversation

AbdlrahmanSaberAbdo
Copy link
Contributor

@AbdlrahmanSaberAbdo AbdlrahmanSaberAbdo commented Oct 18, 2023

This PR addresses an issue where relative paths with leading slashes and dots were not being stripped correctly. The problem arises when the 'path' contains multiple leading slashes like ../../, as the current logic strips it to /../, which is incorrect and prevents the entity from being discovered.

This code modification ensures that any dots followed by slashes are stripped to provide the full path without any dots and slashes at the beginning.

Please note: that the current behavior is such that if you pass ./my/path the result is /my/path However, with the updated code, the result will be my/path This change is made because, in the end, we use the resulting path to check if the source contains the path using endsWith, and the leading slash is unnecessary for this purpose.

Update

I added a slash again after @B4nan clarified a case that we need it in this thread

so the result now for ./my/path will be /my/path

@codecov
Copy link

codecov bot commented Oct 18, 2023

Codecov Report

All modified lines are covered by tests ✅

Files Coverage Δ
packages/core/src/utils/Utils.ts 99.40% <100.00%> (+<0.01%) ⬆️
packages/reflection/src/TsMorphMetadataProvider.ts 100.00% <100.00%> (ø)

📢 Thoughts on this report? Let us know!.

@B4nan B4nan merged commit 8a635c7 into mikro-orm:master Oct 18, 2023
13 checks passed
@B4nan
Copy link
Member

B4nan commented Oct 18, 2023

Thanks!

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.

None yet

2 participants