Add Windows Support #138
Add Windows Support #138
Conversation
…ef, add includes, fix namespace,
…so fix up module name
…ign with main repro example app name
…e.json, and gitignores
|
Will address all the suggested changes Monday or Tuesday. Not around this weekend. |
Co-authored-by: Alexander Sklar <asklar@microsoft.com>
|
@zoontek no rush or anything but just wanted to toss a FYI. I updated all PR suggested changes from @asklar and have tested example app project of what is checked in as of this comment. Just let me know if there are any further suggested changes I can do or change. Thanks! 👍 |
All good, I'll address these new ones soon! Thanks again for looking at it some more. Ill make your suggested changes and I see a few other things myself I'd like to clean up a bit more. The code was quite old/unchanged for us, so it's great to try and get this in a much better state before merging. |
|
It is not ready yet. I need to address a few more things. Just some real life things came up for me and I haven't been able to resolve them yet. Hopefully should be able to super soon. I will @ you when I am ready again. |
|
I have now fixed and/or responded to all of @asklar comments. I have updated last few things and I believe am ready for another look now. I have no further plans for changes unless any one has more suggestions or thoughts. We can wait a little bit in case anyone else wants to add anything before @zoontek gets a VM ready. If there is anything I can do to make it easier for you (e.g. take screenshots/video of anything specific). Just let me know. |
package.json
Outdated
| "localize", | ||
| "localization", | ||
| "l20n" | ||
| ], | ||
| "scripts": { | ||
| "format": "prettier '**/*.{js,json,md,ts,tsx}' --write", | ||
| "prepare": "bob build" | ||
| "prepare": "bob build", | ||
| "windows": "react-native run-windows" |
There was a problem hiding this comment.
Shouldn't this be in the /example app?
There was a problem hiding this comment.
It does sound reasonable, I think I'll just remove this. I believe this was auto added by some script. I never use these helper script on windows, since VS has such a much better flow without it.
Fixed: removed it.
|
LGTM 👍 I trust you guy since I don't own a Windows computer and I can't use virtualization anymore (Apple M1) |
Summary
Add Windows support to react-native-localize #137
What issues does the pull request solve? Please tag them so that they will get automatically closed once the PR is merged
What is the feature? (if applicable)
It adds support for Windows.
How did you implement the solution?
By adding the appropriate changes to support react-native-windows.
What areas of the library does it impact?
Only additive, it shouldn't impact any existing areas.
Test Plan
yarn pod-updatefrom package.json's post install since that doesn't work on windows. But I didn't want to break or modify any existing flows.yarn start-metroWhat's required for testing (prerequisites)?
Windows machine with recent version of node(yarn) and visual studio 2019.
What are the steps to reproduce (after prerequisites)?
Please see above "Test Plan" for how to run example.
Compatibility
Checklist
README.mdCHANGELOG.md.example/App.js)Note:
Missing support for event listener trigger of localizationDidChange. Someone from my team will investigate and PR that in future PR.