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

bump unstable versions by at least a patch #1389

Merged
merged 1 commit into from
Apr 4, 2023

Conversation

mansona
Copy link
Member

@mansona mansona commented Mar 30, 2023

I was having some issues where pnpm was picking up some wrong versions during an ember-try run and I was pulling my hair out 🙃

I noticed that @embroider/compat version 2.1.1 was being used instead of the version 2.1.1-unstable.00ec2e7 that was specified in the package.json. No matter what I did I couldn't get pnpm to pick up correct version. Then I noticed that we're using carat dependencies in the package json on ember-try ever since #1328 and that seems to have caused my issue.

Apparently the version ^2.1.1-unstable.00ec2e7 is satisfied by 2.1.1 😭

Screenshot 2023-03-30 at 17 09 43

This PR will make sure that all new packages that are deployed with the unstable packages will be at least one step higher and won't match any more

@NullVoxPopuli
Copy link
Collaborator

NullVoxPopuli commented Mar 30, 2023

I didn't believe it, but here we are: https://runkit.com/nullvoxpopuli/semver-satisfies-unstable

const semver = require('semver');

const unstable = `2.1.1-unstable.00ec2e7`;
const stable = `2.1.1`;
const range = `^${unstable}`;

console.log(`does ${stable} satisfy range: ${range} :: ${semver.satisfies(stable, range)}`);

(prints true)

If we change the version around so that semver.corce doesn't drop the -unstable tag, unstable-00ec2e7_2.1.1, we get an expected false -- I don't recall if this is a valid version on npm though (I would be surprised if it is)

@NullVoxPopuli
Copy link
Collaborator

We could also get rid of the ^, and that works + also feels less complex: https://runkit.com/nullvoxpopuli/semver-satisfies-unstable

"does 2.1.1 satisfy range: ^2.1.1-unstable.00ec2e7 :: true"
"does 2.1.1 satisfy range: ~2.1.1-unstable.00ec2e7 :: true"
"does 2.1.1 satisfy range: 2.1.1-unstable.00ec2e7 :: false"

Copy link
Collaborator

@NullVoxPopuli NullVoxPopuli left a comment

Choose a reason for hiding this comment

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

Discussed on the call today, and there is more to do (esp around test-setup capabilities), but those things can be separate PRs! 🎉

nice work!!

@ef4 ef4 merged commit 55b12fe into embroider-build:main Apr 4, 2023
@mansona mansona deleted the bump-unstable branch April 4, 2023 16:20
This was referenced May 2, 2023
@ef4 ef4 added the internal label May 11, 2023
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.

None yet

4 participants