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 inconsistent hashes between Windows and POSIX systems #28

Merged
merged 1 commit into from Aug 13, 2018
Merged

Fix inconsistent hashes between Windows and POSIX systems #28

merged 1 commit into from Aug 13, 2018

Conversation

sharkykh
Copy link

@sharkykh sharkykh commented Aug 10, 2018

What kind of change does this PR introduce?
Bugfix

Did you add tests for your changes?
I did not add tests for my changes, but I did verify the fix works by making my webpack bundle on a Linux system, getting one ID or hash, then running the same files and configuration on a Windows system and getting a different hash.

If relevant, did you update the README?
No.

Summary
When using this loader on a project where we have Windows, Linux (and maybe OS X) users,
I discovered we were getting different bundles if one user bundled using a POSIX system, and another bundled the same files on a Windows system.
I believe I only encountered this issue when bundling in development mode.
Edit: You could say it's related to #19. It fixed the issue for different devices using the same system type. This PR fixes the same issue for different devices using different system types.

Does this PR introduce a breaking change?
I do not believe it is a breaking change.

Other information
I don't have an issue to link, however, I used this PR to test my fix.
It includes a fix for vue-loader as well, which I will open a PR for, soon.

This is a simple issue, but if you think a reproduction setup is needed to see the issue in action, let me know and I'll it set up.

@sharkykh sharkykh closed this Aug 10, 2018
@sharkykh sharkykh reopened this Aug 10, 2018
@sharkykh
Copy link
Author

I don't think the tests have failed due to anything I did.

@sharkykh
Copy link
Author

sharkykh commented Aug 12, 2018

Okay, the CI error is caused because it uses npm install instead of yarn install, so it installed the latest version of jsdom (11.12.0), instead of the one in yarn.lock (11.6.2), and that introduced the bug.

I can fix the CI config on this branch, if need be.
But my other PR, #29, to update the CI config to version 2.0, also fixes the error.

@yyx990803 yyx990803 merged commit cf8b6e8 into vuejs:master Aug 13, 2018
@yyx990803
Copy link
Member

Thanks!

@sharkykh sharkykh deleted the fix-inconsistent-hash branch August 13, 2018 23:14
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