fix(android): fix JS error when RN version is a github URL #38
Conversation
package.json
Outdated
| "dependencies": { | ||
| "babel-runtime": "^6.20.0", | ||
| "compare-versions": "^3.0.1", | ||
| "inquirer": "^2.0.0", | ||
| "is-valid-path": "^0.1.1", | ||
| "jest-cli": "^21.2.1", |
There was a problem hiding this comment.
It was needed in my environment.
|
This is a nice catch. There seems to be some overlap with #37. Maybe we can combine the 2. |
|
@jarretmoses if you want to add the fix and the new test to #37 that is fine with me |
|
@dlowder-salesforce I think separate is fine. Mine is actually a WIP when I can get back to fixing 1 thing. This PR shows a nice way of handling the tests though thanks! |
|
@dlowder-salesforce please fix merge conflicts. |
|
@jarretmoses merge conflicts resolved, tests are passing |
src/file-operations.js
Outdated
| @@ -1,6 +1,7 @@ | |||
| const path = require("path"); | |||
| const fs = require("mz/fs"); | |||
| const compareVersions = require("compare-versions"); | |||
| const validate = require("compare-versions"); | |||
There was a problem hiding this comment.
@dlowder-salesforce There looks to be a duplicate import here. This package is being imported above.
src/file-operations.js
Outdated
| @@ -17,9 +17,15 @@ function readFile(file, readDirPath) { | |||
| function parseFile(fileData, { templateName, packageName, app, rnVersion }) { | |||
| let kotlinPackage; | |||
| let javaPackage; | |||
|
|
|||
| var version = rnVersion; | |||
| debugger; | |||
There was a problem hiding this comment.
@dlowder-salesforce this should be removed
| // TODO: figure out a better way to handle one off breaking changes | ||
| if (rnVersion && compareVersions(rnVersion, "0.47.2") < 0) { | ||
| if (version && compareVersions(version, "0.47.2") < 0) { |
There was a problem hiding this comment.
@dlowder-salesforce I am realizing that your version check just validates a github repo as a valid source, however you don't get the actual version from it. How will compareVersions work since I don't see support for github repos in the package.
There was a problem hiding this comment.
Actually a github repo string will fail the validation check, so version will be set to null in that case.
There was a problem hiding this comment.
I see...so I am actually wondering if this PR will cover this issue too and just needs a test https://github.com/peggyrayzis/react-native-create-bridge/pull/37/files
There was a problem hiding this comment.
Yeah that looks like a similar fix
There was a problem hiding this comment.
@dlowder-salesforce Since there are redundancies Im going to add add you test into the other PR and make sure it is still working as expected. I'll add you as a reviewer when it's done.
There was a problem hiding this comment.
@jarretmoses sounds good, I'll close this PR then
What:
In
file-operations.jsparseFile(), validate the RN version string before doing the version check.Why:
A developer using this tool will see a crash if importing react-native directly from github instead of using a release version.
How:
See changes in
file-operations.jsChecklist: