Skip to content

rnpm-plugin-windows fails when using npm #3998

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.

Sign up for GitHub

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

Merged
acoates-ms merged 3 commits into from
Jan 31, 2020
Merged

rnpm-plugin-windows fails when using npm #3998

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
33f9509
Fix CLI dependency
acoates-ms Jan 30, 2020
2284553
Change files
acoates-ms Jan 30, 2020
f599ccd
Wrong version in package.json
acoates-ms Jan 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions change/rnpm-plugin-windows-2020-01-30-13-37-04-npmcli.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"comment": "Fix CLI dependency",
"packageName": "rnpm-plugin-windows",
"email": "acoates@microsoft.com",
"commit": "33f9509e627c111fa5582e11c78d0b00a3b6ce05",
"dependentChangeType": "patch",
"date": "2020-01-30T21:37:04.338Z"
}
1 change: 1 addition & 0 deletions current/local-cli/rnpm/windows/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"extract-zip": "^1.6.7",
"fs-extra": "^7.0.1",
"npm-registry": "^0.1.13",
"prompts": "^2.3.0",
"request": "^2.88.0",
"semver": "^6.1.1",
"valid-url": "^1.0.9"
Expand Down
4 changes: 2 additions & 2 deletions current/local-cli/rnpm/windows/src/common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ const getInstallPackage = function (version, tag, useStable) {
}

if (validVersion) {
return Promise.resolve(`${packageToInstall}@${resultVersion}`);
return Promise.resolve(`${packageToInstall}@${version}`);
} else if (validRange) {
return getMatchingVersion(version, tag, useStable)
.then(resultVersion => `${packageToInstall}@${resultVersion}`);
Expand Down Expand Up @@ -150,5 +150,5 @@ module.exports = {
getInstallPackage,
getReactNativeVersion,
getReactNativeAppName,
isGlobalCliUsingYarn
isGlobalCliUsingYarn,
};
67 changes: 44 additions & 23 deletions current/local-cli/rnpm/windows/src/windows.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ const Common = require('./common');
const chalk = require('chalk');
const execSync = require('child_process').execSync;
const path = require('path');
const prompt = require('@react-native-community/cli/build/tools/generator/promptSync').default();
Copy link
Copy Markdown
Contributor

@kmelmon kmelmon Jan 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

vnext\local-cli\generator-common\index.js also require's and uses this module. Does that need to be changed as well? prompts looks better anyway.

Copy link
Copy Markdown
Contributor Author

@acoates-ms acoates-ms Jan 30, 2020

Choose a reason for hiding this comment

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

Thats in react-native-windows which declares the cli dep already.

Copy link
Copy Markdown
Contributor

@kmelmon kmelmon Jan 30, 2020

Choose a reason for hiding this comment

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

That dependency is about to be removed when we move to 61

Copy link
Copy Markdown
Contributor Author

@acoates-ms acoates-ms Jan 30, 2020

Choose a reason for hiding this comment

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

If its removed the generator will have to be updated to not use it.

I expect that over time we will require the cli dependency as we work to properly implement auto-linking etc -- so maybe we should just keep the dependency and update it to match the same version range that react-native itself uses in 61.

const prompts = require('prompts');
const semver = require('semver');

const REACT_NATIVE_WINDOWS_GENERATE_PATH = function() {
return path.resolve(
Expand All @@ -20,30 +21,50 @@ const REACT_NATIVE_WINDOWS_GENERATE_PATH = function() {
);
};

module.exports = function (config, args, options) {
const name = args[0] ? args[0] : Common.getReactNativeAppName();
const ns = options.namespace ? options.namespace : name;
const version = options.windowsVersion ? options.windowsVersion : Common.getReactNativeVersion();

let template = options.template;
if (!template) {
console.log("What version of react-native-windows would you like to install? Choose one of: legacy, latest [default]:");
template = prompt();
if (template === '') {
template = 'vnext'
module.exports = async function (config, args, options) {
try {
const name = args[0] ? args[0] : Common.getReactNativeAppName();
const ns = options.namespace ? options.namespace : name;
let version = options.windowsVersion ? options.windowsVersion : Common.getReactNativeVersion();

let template = options.template;
if (!template) {
const validVersion = semver.valid(version);
const validRange = semver.validRange(version);
// If the RN version is >= 0.60 we know they have to use vnext
if ((validVersion && semver.gte(validVersion, '0.60')) ||
(!validVersion && validRange && semver.ltr('0.59.1000', validRange))) {
template = 'vnext';
}

// If the RN version is >= 0.59 then we need to query which version the user wants
if (!template && ((validVersion && semver.gte(validVersion, '0.59.0')) ||
(!validVersion && validRange && semver.ltr('0.58.1000', validRange)))) {
template = (await prompts({
type: 'select',
name: 'template',
message: 'What version of react-native-windows would you like to install?',
choices: [
{ value: 'vnext', title: ' Latest - High performance react-native-windows built on a shared C++ core from facebook (supports C++ or C#).' },
{ value: 'legacy', title: ' Legacy - Older react-native-windows implementation - (C# only, react-native <= 0.59 only)' },
],
})).template;
}
}
}

return Common.getInstallPackage(version, template, true)
.then(rnwPackage => {
console.log(`Installing ${rnwPackage}...`);
const pkgmgr = Common.isGlobalCliUsingYarn(process.cwd()) ? 'yarn add' : 'npm install --save';
let rnwPackage = await Common.getInstallPackage(version, template, !!template);

const execOptions = options.verbose ? { stdio: 'inherit' } : {};
execSync(`${pkgmgr} ${rnwPackage}`, execOptions);
console.log(chalk.green(`${rnwPackage} successfully installed.`));
console.log(`Installing ${rnwPackage}...`);
const pkgmgr = Common.isGlobalCliUsingYarn(process.cwd()) ? 'yarn add' : 'npm install --save';

const generateWindows = require(REACT_NATIVE_WINDOWS_GENERATE_PATH());
generateWindows(process.cwd(), name, ns, options);
}).catch(error => console.error(chalk.red(error.message)));
const execOptions = options.verbose ? { stdio: 'inherit' } : {};
execSync(`${pkgmgr} ${rnwPackage}`, execOptions);
console.log(chalk.green(`${rnwPackage} successfully installed.`));

const generateWindows = require(REACT_NATIVE_WINDOWS_GENERATE_PATH());
generateWindows(process.cwd(), name, ns, options);
} catch (error) {
console.error(chalk.red(error.message));
console.error(error);
}
};