Skip to content

Commit

Permalink
Fix HMR on Windows
Browse files Browse the repository at this point in the history
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
  • Loading branch information
janicduplessis authored and Facebook Github Bot 6 committed Apr 5, 2016
1 parent bef175a commit c61100d
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 9 deletions.
6 changes: 6 additions & 0 deletions babel-preset/configs/hmr.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ module.exports = function(options, filename) {
var transform = filename
? './' + path.relative(path.dirname(filename), transformPath) // packager can't handle absolute paths
: hmrTransform;

// Fix the module path to use '/' on Windows.
if (path.sep === '\\') {
transform = transform.replace(/\\/g, '/');
}

return {
plugins: resolvePlugins([
[
Expand Down
19 changes: 10 additions & 9 deletions packager/react-packager/src/Bundler/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,23 +189,24 @@ class Bundler {
);
}

_hmrURL(prefix, platform, extensionOverride, path) {
const matchingRoot = this._projectRoots.find(root => path.startsWith(root));
_hmrURL(prefix, platform, extensionOverride, filePath) {
const matchingRoot = this._projectRoots.find(root => filePath.startsWith(root));

if (!matchingRoot) {
throw new Error('No matching project root for ', path);
throw new Error('No matching project root for ', filePath);
}

const extensionStart = path.lastIndexOf('.');
let resource = path.substring(
// Replaces '\' with '/' for Windows paths.
if (path.sep === '\\') {
filePath = filePath.replace(/\\/g, '/');
}

const extensionStart = filePath.lastIndexOf('.');
let resource = filePath.substring(
matchingRoot.length,
extensionStart !== -1 ? extensionStart : undefined,
);

const extension = extensionStart !== -1
? path.substring(extensionStart + 1)
: null;

return (
prefix + resource +
'.' + extensionOverride + '?' +
Expand Down

8 comments on commit c61100d

@cdreier
Copy link

Choose a reason for hiding this comment

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

are there any other things to know? i just tested on "react-native": "^0.25.0-rc" - but i still get that error when activating hmr

hmr

@janicduplessis
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have you tried clearing the packager cache so all the babel transforms are run again? You can do npm start -- --reset-cache

@janicduplessis
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually part of the fix is in the babel-preset-react-native package and I'm not sure if we have published a new version since, will make sure we do.

@janicduplessis
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Double checked and we haven't published a new version since my fix, just submitted a PR to do so. Once it lands and the babel-preset is published to npm you should just have to run npm install and it will work.

@cdreier
Copy link

Choose a reason for hiding this comment

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

Thanks a lot for your help! Any forecast how long this could take?

@janicduplessis
Copy link
Contributor Author

@janicduplessis janicduplessis commented on c61100d Apr 21, 2016

Choose a reason for hiding this comment

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

PR just got merged, just need someone to publish the preset to npm, should be done by tomorrow.

@janicduplessis
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cdreier Should be fixed now :)

@cdreier
Copy link

Choose a reason for hiding this comment

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

@janicduplessis just testet and everything is working fine again! im happy :-) thanks again!

Please sign in to comment.