Skip to content

Conversation

@youedd
Copy link
Contributor

@youedd youedd commented Apr 15, 2023

We were facing issues with the appcenter cli after migrating our react-native project to a mono-repository.

The cli is looking for react-native package is in the project node_modules folder instead of the root node_modules, which leads to errors.

This PR aims to solve that by using node's require.resolve to find the package path.

@youedd youedd requested a review from a team as a code owner April 15, 2023 11:38
@DmitriyKirakosyan
Copy link
Contributor

Hi @youedd , thank you for the contribution! If I understand correctly, this solution will work only if appcenter-cli is added to your react-native project as a dependency. What if we use it as a globally installed tool?

@youedd
Copy link
Contributor Author

youedd commented Apr 18, 2023

Hello @DmitriyKirakosyan,

Installing the appcenter-cli globally or locally shouldn't have an impact.

The node --print "require.resolve('react-native/package.json')" is executed in the cwd, which will always be the root folder of the react native package since the codepush release-react command enforce that.

public async run(client: AppCenterClient): Promise<CommandResult> {
if (!getReactNativeVersion()) {
return failure(ErrorCodes.InvalidParameter, "The project in the CWD is not a React Native project.");
}

export function getReactNativeVersion(): string {
let packageJsonFilename;
let projectPackageJson;
try {
packageJsonFilename = path.join(process.cwd(), "package.json");
projectPackageJson = JSON.parse(fs.readFileSync(packageJsonFilename, "utf-8"));
} catch (error) {
throw new Error(
`Unable to find or read "package.json" in the CWD. The "release-react" command must be executed in a React Native project folder.`
);
}
const projectName: string = projectPackageJson.name;
if (!projectName) {
throw new Error(`The "package.json" file in the CWD does not have the "name" field set.`);
}
return (
(projectPackageJson.dependencies && projectPackageJson.dependencies["react-native"]) ||
(projectPackageJson.devDependencies && projectPackageJson.devDependencies["react-native"])
);
}

In case the resolved path does not exist, it will fallback to the current logic.

function getReactNativePackagePath(): string {
const result = childProcess.spawnSync("node", ["--print", "require.resolve('react-native/package.json')"]);
const packagePath = path.dirname(result.stdout.toString());
if (directoryExistsSync(packagePath)) {
return packagePath;
}
return path.join("node_modules", "react-native");

@DmitriyKirakosyan
Copy link
Contributor

@youedd thank you for the clarification. You are right.

However, the fallback doesn't work. If the process throws an error, it will return empty string in result.stdout and path.dirname("") returns ".". Then it will confirm that the path "." exists and will return it. I think that in addition to checking if the path exists, we also need to check that result.status is 0.

Also, probably worth adding a timeout to spawnSync call, in case it freezes for some reason.

@AnatolyPristensky
Copy link
Contributor

Merged with a needful changes as #2305

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants