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

fix: Execute npm version with --workspaces=false #512

Closed
wants to merge 1 commit into from

Conversation

aarne
Copy link

@aarne aarne commented Sep 11, 2022

Some tools like https://github.com/qiwi/multi-semantic-release wrap semantic release. Unfortunately with newer versions of npm people started getting EUNSUPPORTEDPROTOCOL errors during version locking as described here:

This change just adds --workspaces=false to npm command during versioning. This parameter has not side effects on normal repos but fixes the issue for monorepo setups

@travi
Copy link
Member

travi commented Sep 12, 2022

while we dont want to prevent such tools from extending semantic-release, keep in mind that we dont officially support monorepos. therefore, we avoid adding behavior here that is specifically for supporting monorepos. command-line flags for npm can also be added to your .npmrc file within your project and is our recommendation for situations like this one.

@manast
Copy link

manast commented Sep 12, 2022

I created a .npmrc with this content:

workspaces=false

however when I try to make the release I get this error now:

npm ERR! Can not use --no-workspaces and --workspace at the same time

Any ideas?

@aarne
Copy link
Author

aarne commented Sep 13, 2022

we run a simple command npm version 1.0.0 --no-git-tag-version --allow-same-version in individual packages.

  1. usually it fails with EUNSUPPORTEDPROTOCOL
  2. with --workspaces=false (this pr) it works as expected
  3. with suggested modification to .npmrc we get an error Can not use --no-workspaces and --workspace at the same time

We can just create a new fork and publish sr-npm-monorepo-safe but that sounds a bit too much for such a simple change :)

Most monorepo tools will allow you to switch out the implementation in release config

    "plugins": [
        "@semantic-release/commit-analyzer",
        "sr-npm-monorepo-safe",
    ]

@manast
Copy link

manast commented Sep 13, 2022

while we dont want to prevent such tools from extending semantic-release, keep in mind that we dont officially support monorepos.

@travi it could even be argued that since you do not support monorepos, adding workspace=false as default makes more sense than letting it be.

@manast
Copy link

manast commented Sep 13, 2022

@aarne I made the change of this PR in a custom fork and the error I am getting now is this one:

npm ERR! Cannot read properties of null (reading 'matches')

Could not find any reference to this using google either. Locally I do not receive this error however, only when running as github actions.

@aarne
Copy link
Author

aarne commented Sep 13, 2022

For me it's working with the mentioned modifications with @qiwi/multi-semantic-release. Are you sure this error is related to this change and this repo or its a general question? We can sync offline from here if its general to keep this PRs clear of unrelated discussions.

@manast
Copy link

manast commented Sep 13, 2022

@arne I think this discussion is still relevant for this PR. I discovered that the last error is gone if I downgrade Node to version 14 instead of 16.

@aarne
Copy link
Author

aarne commented Sep 13, 2022

I think its not related to node version but npm version that comes with it ... its broken since npm/cli@723a091

if you take the latest npm and run version with --workspaces=false it works as expected

actually anything that has npm version below 8.5 should work. Try node version 16.14 for example

@aarne
Copy link
Author

aarne commented Sep 16, 2022

@travi Please let us know if it makes sense to wait for this PR or we should be looking for another alternative?

@travi
Copy link
Member

travi commented Sep 16, 2022

other semantic-release users have been using workspaces successfully without changes to semantic-release, but with only changing .npmrc. i am not yet convinced that the situation you are describing is not in the same category. please see the following conversations:

it could even be argued that since you do not support monorepos, adding workspace=false as default makes more sense than letting it be.

the error that you mention above shows why this is not the case. if we include arbitrary flags in the commands executed by core, it increases the chances of conflicts with other flags needed by some projects

3. with suggested modification to .npmrc we get an error Can not use --no-workspaces and --workspace at the same time

the conflicting flag is coming from something in your project, not semantic-release core. i suggest you find what is introducing the additional flag and determine if that flag is necessary.

again, i want to highlight that semantic-release itself does not support monorepos. a big part of this stance is because we do not have the bandwidth to dig into problems that are out of scope in addition to the other project priorities that are out in scope. we dont want to prevent other tools from extending semantic-release to add functionality like this, but the investigation and work needs to be done by the community around those tools. for us to even consider making a change to core that is specifically to enable monorepo usage, it needs to be proven that a change is needed that cannot be accomplished outside of core and that the change does not introduce problems for projects that are not using such an approach.

@aarne
Copy link
Author

aarne commented Sep 17, 2022

@travi You are correct, adding workspaces-update = false avoids the configuration conflict and resolves the issue with version command. The additional references you sent helped, thank you.

I will update the still open bugs in other projects with instructions so others will find the solution also.

@aarne aarne closed this Sep 17, 2022
@travi
Copy link
Member

travi commented Sep 19, 2022

thank you for following up to let us know and for updating the other issues, @aarne. would you be interested in helping us make an update to our docs to help others find this information?

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