Skip to content

Commit be0d87d

Browse files
tido64tido64
authored
fix(windows): fix error parsing Hermes engine version (#523)
1 parent d534704 commit be0d87d

9 files changed

Lines changed: 250 additions & 30 deletions

File tree

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@
163163
}
164164
},
165165
"jest": {
166+
"collectCoverage": true,
166167
"roots": [
167168
"test"
168169
]

‎react-native.config.js‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,15 +71,13 @@ module.exports = {
7171
name: "init-test-app",
7272
description: "Initializes a new test app project",
7373
func: (_argv, _config, { destination, name, platform }) => {
74-
const {
75-
configure,
76-
getTargetReactNativeVersion,
77-
} = require("./scripts/configure");
74+
const { version: targetVersion } = require("react-native/package.json");
75+
const { configure } = require("./scripts/configure");
7876
configure({
7977
name,
8078
packagePath: destination,
8179
testAppPath: __dirname,
82-
targetVersion: getTargetReactNativeVersion(),
80+
targetVersion,
8381
platforms: sanitizePlatformChoice(platform),
8482
flatten: true,
8583
force: true,

‎scripts/configure.js‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,7 @@ function gatherConfig(params) {
603603

604604
const dependencies = getDependencies && getDependencies(params);
605605
if (!dependencies) {
606+
/* istanbul ignore next */
606607
return config;
607608
}
608609

@@ -667,16 +668,6 @@ function getAppName(packagePath) {
667668
return "ReactTestApp";
668669
}
669670

670-
/**
671-
* Retrieves the version of React Native to target.
672-
* @returns {string}
673-
*/
674-
function getTargetReactNativeVersion() {
675-
const manifestPath = require.resolve("react-native/package.json");
676-
const { version } = readJSONFile(manifestPath);
677-
return /** @type {string} */ (version);
678-
}
679-
680671
/**
681672
* Returns whether destructive operations will be required.
682673
* @param {string} packagePath
@@ -849,7 +840,7 @@ function configure(params) {
849840
if (require.main === module) {
850841
/** @type {Platform[]} */
851842
const platformChoices = ["android", "ios", "macos", "windows"];
852-
const targetVersion = getTargetReactNativeVersion();
843+
const { version: targetVersion } = require("react-native/package.json");
853844

854845
require("yargs").usage(
855846
"$0 [options]",
@@ -916,7 +907,6 @@ exports.gatherConfig = gatherConfig;
916907
exports.getAppName = getAppName;
917908
exports.getConfig = getConfig;
918909
exports.getPlatformPackage = getPlatformPackage;
919-
exports.getTargetReactNativeVersion = getTargetReactNativeVersion;
920910
exports.isDestructive = isDestructive;
921911
exports.isInstalled = isInstalled;
922912
exports.join = join;

‎scripts/init.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88
// @ts-check
99

1010
(async () => {
11-
const { configure, getTargetReactNativeVersion } = require("./configure");
12-
const targetVersion = getTargetReactNativeVersion();
11+
const { version: targetVersion } = require("react-native/package.json");
12+
const { configure } = require("./configure");
1313

1414
/**
1515
* @type {{

‎test/configure/__snapshots__/gatherConfig.test.js.snap‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,166 @@ module.exports = {
201201
}
202202
`;
203203

204+
exports[`gatherConfig() returns common configuration 1`] = `
205+
Object {
206+
"dependencies": Object {},
207+
"files": Object {
208+
".gitignore": Object {
209+
"source": "example/.gitignore",
210+
},
211+
".watchmanconfig": Object {
212+
"source": "node_modules/react-native/template/_watchmanconfig",
213+
},
214+
"babel.config.js": Object {
215+
"source": "node_modules/react-native/template/babel.config.js",
216+
},
217+
"common/.gitignore": Object {
218+
"source": "example/.gitignore",
219+
},
220+
"common/.watchmanconfig": Object {
221+
"source": "node_modules/react-native/template/_watchmanconfig",
222+
},
223+
"common/babel.config.js": Object {
224+
"source": "node_modules/react-native/template/babel.config.js",
225+
},
226+
"common/metro.config.js": Object {
227+
"source": "example/metro.config.js",
228+
},
229+
"common/react-native.config.js": "const fs = require(\\"fs\\");
230+
const path = require(\\"path\\");
231+
232+
const windowsProjectFile = path.join(
233+
\\"node_modules\\",
234+
\\".generated\\",
235+
\\"windows\\",
236+
\\"ReactTestApp\\",
237+
\\"ReactTestApp.vcxproj\\"
238+
);
239+
240+
module.exports = {
241+
project: {
242+
android: {
243+
sourceDir: \\"android\\",
244+
manifestPath: path.relative(
245+
path.join(__dirname, \\"android\\"),
246+
path.join(
247+
path.dirname(require.resolve(\\"react-native-test-app/package.json\\")),
248+
\\"android\\",
249+
\\"app\\",
250+
\\"src\\",
251+
\\"main\\",
252+
\\"AndroidManifest.xml\\"
253+
)
254+
),
255+
},
256+
ios: {
257+
project: (() => {
258+
const {
259+
packageSatisfiesVersionRange,
260+
} = require(\\"react-native-test-app/scripts/configure\\");
261+
if (
262+
packageSatisfiesVersionRange(
263+
\\"@react-native-community/cli-platform-ios\\",
264+
\\"<5.0.2\\"
265+
)
266+
) {
267+
// Prior to @react-native-community/cli-platform-ios v5.0.0,
268+
// \`project\` was only used to infer \`sourceDir\` and \`podfile\`.
269+
return \\"ios/ReactTestApp-Dummy.xcodeproj\\";
270+
}
271+
272+
// \`sourceDir\` and \`podfile\` detection was fixed in
273+
// @react-native-community/cli-platform-ios v5.0.2 (see
274+
// https://github.com/react-native-community/cli/pull/1444).
275+
return \\"node_modules/.generated/ios/ReactTestApp.xcodeproj\\";
276+
})(),
277+
},
278+
windows: fs.existsSync(windowsProjectFile) && {
279+
sourceDir: \\"windows\\",
280+
solutionFile: \\"Test.sln\\",
281+
project: {
282+
projectFile: path.relative(
283+
path.join(__dirname, \\"windows\\"),
284+
windowsProjectFile
285+
),
286+
},
287+
},
288+
},
289+
};
290+
",
291+
"metro.config.js": Object {
292+
"source": "example/metro.config.js",
293+
},
294+
"react-native.config.js": "const fs = require(\\"fs\\");
295+
const path = require(\\"path\\");
296+
297+
const windowsProjectFile = path.join(
298+
\\"node_modules\\",
299+
\\".generated\\",
300+
\\"windows\\",
301+
\\"ReactTestApp\\",
302+
\\"ReactTestApp.vcxproj\\"
303+
);
304+
305+
module.exports = {
306+
project: {
307+
android: {
308+
sourceDir: \\"android\\",
309+
manifestPath: path.relative(
310+
path.join(__dirname, \\"android\\"),
311+
path.join(
312+
path.dirname(require.resolve(\\"react-native-test-app/package.json\\")),
313+
\\"android\\",
314+
\\"app\\",
315+
\\"src\\",
316+
\\"main\\",
317+
\\"AndroidManifest.xml\\"
318+
)
319+
),
320+
},
321+
ios: {
322+
project: (() => {
323+
const {
324+
packageSatisfiesVersionRange,
325+
} = require(\\"react-native-test-app/scripts/configure\\");
326+
if (
327+
packageSatisfiesVersionRange(
328+
\\"@react-native-community/cli-platform-ios\\",
329+
\\"<5.0.2\\"
330+
)
331+
) {
332+
// Prior to @react-native-community/cli-platform-ios v5.0.0,
333+
// \`project\` was only used to infer \`sourceDir\` and \`podfile\`.
334+
return \\"ios/ReactTestApp-Dummy.xcodeproj\\";
335+
}
336+
337+
// \`sourceDir\` and \`podfile\` detection was fixed in
338+
// @react-native-community/cli-platform-ios v5.0.2 (see
339+
// https://github.com/react-native-community/cli/pull/1444).
340+
return \\"node_modules/.generated/ios/ReactTestApp.xcodeproj\\";
341+
})(),
342+
},
343+
windows: fs.existsSync(windowsProjectFile) && {
344+
sourceDir: \\"windows\\",
345+
solutionFile: \\"Test.sln\\",
346+
project: {
347+
projectFile: path.relative(
348+
path.join(__dirname, \\"windows\\"),
349+
windowsProjectFile
350+
),
351+
},
352+
},
353+
},
354+
};
355+
",
356+
},
357+
"oldFiles": Array [],
358+
"scripts": Object {
359+
"start": "react-native start",
360+
},
361+
}
362+
`;
363+
204364
exports[`gatherConfig() returns configuration for a single platform 1`] = `
205365
Object {
206366
"dependencies": Object {},

‎test/configure/gatherConfig.test.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,12 @@ describe("gatherConfig()", () => {
1414
expect(gatherConfig(mockParams())).toMatchSnapshot();
1515
});
1616

17+
test("returns common configuration", () => {
18+
expect(
19+
gatherConfig(mockParams({ platforms: ["common"] }))
20+
).toMatchSnapshot();
21+
});
22+
1723
test("returns configuration for a single platform", () => {
1824
expect(gatherConfig(mockParams({ platforms: ["ios"] }))).toMatchSnapshot();
1925
});

‎test/configure/packageSatisfiesVersionRange.test.js‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
//
2+
// Copyright (c) Microsoft Corporation
3+
//
4+
// This source code is licensed under the MIT license found in the
5+
// LICENSE file in the root directory of this source tree.
6+
//
7+
// @ts-check
8+
9+
describe("packageSatisfiesVersionRange()", () => {
10+
const { packageSatisfiesVersionRange } = require("../../scripts/configure");
11+
12+
test("returns true when a package satisfies version range", () => {
13+
expect(packageSatisfiesVersionRange("..", "^0.0.1-0")).toBe(true);
14+
expect(packageSatisfiesVersionRange("..", "~0.0.1-0")).toBe(true);
15+
});
16+
17+
test("returns false when a package does not satisfy version range", () => {
18+
expect(packageSatisfiesVersionRange("..", ">=0.0.1")).toBe(false);
19+
expect(packageSatisfiesVersionRange("..", "^0.0.0")).toBe(false);
20+
});
21+
});

‎test/windows-test-app/getHermesVersion.test.js‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
//
2+
// Copyright (c) Microsoft Corporation
3+
//
4+
// This source code is licensed under the MIT license found in the
5+
// LICENSE file in the root directory of this source tree.
6+
//
7+
// @ts-check
8+
9+
jest.mock("fs");
10+
11+
/**
12+
* Returns a property sheet with specified Hermes version.
13+
* @param {string} version
14+
* @returns {string}
15+
*/
16+
function jsEngineProps(version) {
17+
return `
18+
<?xml version="1.0" encoding="utf-8"?>
19+
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
20+
<PropertyGroup>
21+
<HermesVersion Condition="'$(HermesVersion)' == ''">${version}</HermesVersion>
22+
</PropertyGroup>
23+
</Project>
24+
`;
25+
}
26+
27+
describe("getHermesVersion", () => {
28+
const path = require("path");
29+
const { mockFiles } = require("../mockFiles");
30+
const { getHermesVersion } = require("../../windows/test-app");
31+
32+
afterEach(() => mockFiles());
33+
34+
test("returns the required Hermes version", () => {
35+
["0.7.2", "0.8.0-ms.0", "0.9.0-ms.1"].forEach((version) => {
36+
mockFiles([
37+
path.join("react-native-windows", "PropertySheets", "JSEngine.props"),
38+
jsEngineProps(version),
39+
]);
40+
expect(getHermesVersion("react-native-windows")).toBe(version);
41+
});
42+
});
43+
});

‎windows/test-app.js‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ function getHermesVersion(rnwPath) {
274274
"JSEngine.props"
275275
);
276276
const props = fs.readFileSync(jsEnginePropsPath, textFileReadOptions);
277-
const m = props.match(/<HermesVersion.*?>([.\w]+)<\/HermesVersion>/);
277+
const m = props.match(/<HermesVersion.*?>(.+?)<\/HermesVersion>/);
278278
return m && m[1];
279279
}
280280

@@ -602,14 +602,15 @@ if (require.main === module) {
602602
}
603603
).argv;
604604
} else {
605-
exports["copyAndReplace"] = copyAndReplace;
606-
exports["findNearest"] = findNearest;
607-
exports["findUserProjects"] = findUserProjects;
608-
exports["generateSolution"] = generateSolution;
609-
exports["getBundleResources"] = getBundleResources;
610-
exports["getPackageVersion"] = getPackageVersion;
611-
exports["getVersionNumber"] = getVersionNumber;
612-
exports["parseResources"] = parseResources;
613-
exports["replaceContent"] = replaceContent;
614-
exports["toProjectEntry"] = toProjectEntry;
605+
exports.copyAndReplace = copyAndReplace;
606+
exports.findNearest = findNearest;
607+
exports.findUserProjects = findUserProjects;
608+
exports.generateSolution = generateSolution;
609+
exports.getBundleResources = getBundleResources;
610+
exports.getHermesVersion = getHermesVersion;
611+
exports.getPackageVersion = getPackageVersion;
612+
exports.getVersionNumber = getVersionNumber;
613+
exports.parseResources = parseResources;
614+
exports.replaceContent = replaceContent;
615+
exports.toProjectEntry = toProjectEntry;
615616
}

0 commit comments

Comments
 (0)