-
Notifications
You must be signed in to change notification settings - Fork 34
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 wrapper to work on Windows #522
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving, but please reverse spread operator change
@@ -97,7 +101,7 @@ function transformWrapper({ filename, src, ...rest }) { | |||
} | |||
} | |||
|
|||
return transform({ filename, src, ...rest }); | |||
return transform({ filename, src, options, plugins }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this change here? I actually fixed one issue in another transformer a while ago that was assuming only three elements in that object exists while plugins
was added afterwards to the specification. Given we don't use all of them it'd be more future proof to have the ellipsis here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, can you provide the test plan here? |
This PR adds a missing import in the previous PR #522 and changes `/` path separator to platform-specific in`babel_transformer.js`.
We're using filenames which are platform-specific to check what files should be modified. Windows uses backslashes so it wasn't ever matched.
Testing plan
Run any app on windows and check if RN IDE functionality works (inspector, etc).