Skip to content
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.

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

Don't ask to update project if version is unknown #1405

Merged
merged 2 commits into from
Feb 20, 2024

Conversation

mihaiblaga89
Copy link
Contributor

@mihaiblaga89 mihaiblaga89 commented Feb 16, 2024

Description

  • rnv assumes it's a devDep in projects, will trigger a weird update message if it's in dependencies.
[ warn ] [project configure] You are running $rnv v1.0.0-rc.11 against project built with rnv vunknown. This might result in unexpected behaviour!
It is recommended that you run your rnv command with npx prefix: $ npx npx rnv hooks run -x generateDocs . or manually update your devDependencies.rnv version in your package.json.
? What to do next? (Use arrow keys)
❯ Continue and skip updating package.json
  Continue and update package.json
  Upgrade project to 1.0.0-rc.11

Related issues

  • GH issues

Npm releases

n/a

@@ -617,7 +617,7 @@ export const versionCheck = async (c: RnvContext) => {
`versionCheck:rnvRunner:${c.runtime.rnvVersionRunner},rnvProject:${c.runtime.rnvVersionProject}`,
chalk().grey
);
if (c.runtime.rnvVersionRunner && c.runtime.rnvVersionProject) {
if (c.runtime.rnvVersionRunner && c.runtime.rnvVersionProject && c.runtime.rnvVersionRunner !== 'unknown') {
Copy link
Member

Choose a reason for hiding this comment

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

@mihaiblaga89 this workaround looks weird. we're already checking c.runtime.rnvVersionProject which in this case is present. and it get's it from c.files.project?.package?.devDependencies?.rnv

adding c.runtime.rnvVersionRunner !== 'unknown' seems like a ducktape of symptom rather than tackling actual culprit.

we need to understand why is c.files.rnv.package not populated at this point instead
first check should be c.paths.rnv.dir value and what's going on in createRnvContext

can you provide repro case hod did you arrived at this stage

Copy link
Member

Choose a reason for hiding this comment

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

@mihaiblaga89 also add link to related GH issue this PR is meant to solve/fix

Copy link
Contributor Author

@mihaiblaga89 mihaiblaga89 Feb 19, 2024

Choose a reason for hiding this comment

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

@pavjacko that's correct, rnvVersionProject should have been checked for unknown. The problem was that the project I was using this in was not a renative project and had rnv as a dep, not devDep. Added dep possibility and removed the unknown check

@mihaiblaga89 mihaiblaga89 marked this pull request as draft February 19, 2024 07:47
@mihaiblaga89 mihaiblaga89 self-assigned this Feb 19, 2024
@mihaiblaga89 mihaiblaga89 added this to the 1.0 milestone Feb 19, 2024
@mihaiblaga89 mihaiblaga89 marked this pull request as ready for review February 19, 2024 14:26
@pavjacko pavjacko merged commit d80f450 into release/1.0 Feb 20, 2024
3 checks passed
@pavjacko pavjacko deleted the fix/project_unknown_version branch February 20, 2024 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants