Fix HMR on Windows #6678
Fix HMR on Windows #6678
Conversation
| const extensionStart = path.lastIndexOf('.'); | ||
| let resource = path.substring( | ||
| matchingRoot.length, | ||
| extensionStart !== -1 ? extensionStart : undefined, | ||
| ); | ||
|
|
||
| const extension = extensionStart !== -1 |
There was a problem hiding this comment.
unused variable
|
By analyzing the blame information on this pull request, we identified @martinbigio, @davidaurelio and @amasad to be potential reviewers. |
facebook-github-bot
added
GH Review: review-needed
CLA Signed
| @@ -18,6 +18,10 @@ module.exports = function(options, filename) { | |||
| var transform = filename | |||
| ? './' + path.relative(path.dirname(filename), transformPath) // packager can't handle absolute paths | |||
| : hmrTransform; | |||
|
|
|||
| // Fix the path to use '/' on Windows. | |||
| transform = transform.replace(/\\/g, '/'); | |||
There was a problem hiding this comment.
Here the transform used to output require('./..\..\...') which of course didn't work.
There was a problem hiding this comment.
It should check the path separator first,
if (path.sep === '\\') {
transform = transform.replace(/\\/g, '/');
}I think we should move this to a module normalizePath or something, and use it instead of duplicating the code.
|
cc @martinbigio |
|
@janicduplessis can you make changes based on my comment? |
|
@satya164 Oups I forgot to get back to you on this PR :) I don't think we need to make it in a module it's actually pretty rare we need to change backslashes for forward slashes. It's only needed here since we derive a URL and a require path from a file path. I don't mind adding the extra check on path.sep, I just figured the overhead was negligible. |
|
@janicduplessis updated the pull request. |
|
@janicduplessis the same code exists at a different place though |
|
The main issue is that one is in babel-preset and the other one in the packager. Babel preset is actually a different module so I don't think it's possible to require something from the same place. |
|
@janicduplessis Oh. Right. @facebook-github-bot shipit |
facebook-github-bot
added
GH Review: accepted
and removed
GH Review: review-needed
labels
|
Thanks for importing. If you are an FB employee go to Phabricator to review. |
ghost
closed this
in
Summary:Tested HMR on Windows and found 2 small issues related to paths that made it not work. Now it works nicely :) **Test plan (required)** Tested HMR in UIExplorer on Windows. Closes #6678 Differential Revision: D3138379 fb-gh-sync-id: f27cd2fa21f95954685c8c6916d820f41bc187be fbshipit-source-id: f27cd2fa21f95954685c8c6916d820f41bc187be
Summary:Tested HMR on Windows and found 2 small issues related to paths that made it not work. Now it works nicely :) **Test plan (required)** Tested HMR in UIExplorer on Windows. Closes facebook#6678 Differential Revision: D3138379 fb-gh-sync-id: f27cd2fa21f95954685c8c6916d820f41bc187be fbshipit-source-id: f27cd2fa21f95954685c8c6916d820f41bc187be
Summary:Tested HMR on Windows and found 2 small issues related to paths that made it not work. Now it works nicely :) **Test plan (required)** Tested HMR in UIExplorer on Windows. Closes facebook/react-native#6678 Differential Revision: D3138379 fb-gh-sync-id: f27cd2fa21f95954685c8c6916d820f41bc187be fbshipit-source-id: f27cd2fa21f95954685c8c6916d820f41bc187be
Tested HMR on Windows and found 2 small issues related to paths that made it not work. Now it works nicely :)
Test plan (required)
Tested HMR in UIExplorer on Windows.