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: prevent false positive for startsWith(config.root) call #1378

Closed
wants to merge 2 commits into from

Conversation

aleclarson
Copy link
Member

No description provided.

Copy link
Member

@yyx990803 yyx990803 left a comment

Choose a reason for hiding this comment

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

config.root is pre-normalized to posix style. Should just check / instead of path.sep.

@aleclarson
Copy link
Member Author

aleclarson commented Jan 5, 2021

Ah, so that means resolveId hooks should never return Windows paths. Got it.

I guess that also means this line could be path.normalize instead of normalizePath:

url = FS_PREFIX + normalizePath(resolved.id)

@aleclarson
Copy link
Member Author

Ready to merge

@yyx990803
Copy link
Member

windows resolve test is failing due to this change.

@aleclarson
Copy link
Member Author

Then I guess your assumption that resolve returns posix paths is false?

@yyx990803
Copy link
Member

yyx990803 commented Jan 6, 2021

The resolve plugin always returns normalized paths, but it's possible for other plugins to return un-normalized paths. I think it's probably better to call normalizePath once on resolved.id here right after it's resolved before using it.

Actually, we should call normalizePath in pluginContainer's resolveId method before returning the id.

@yyx990803 yyx990803 closed this in da3dfdf Jan 7, 2021
@aleclarson aleclarson deleted the fix/startsWith-root branch June 24, 2021 23:56
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.

2 participants