Open BryanCrotaz opened 3 years ago
However this beta is not marked as latest, so any other addons that require ESA won't work
You can use ember-simple-auth-token
with versions of ember-simple-auth
that are not marked as latest. In the vast majority of the applications I manage, we use an older version of ember-simple-auth
and the latest version of ember-simple-auth-token
. You simply ember install
the specific version of ember-simple-auth
that you want to be used.
and never recommend installation of a beta - that's guaranteed to cause these types of problems.
It should not cause problems. You are implying that you can only use the latest version of a package that another package depends on which is not true. You can use more recent version or older versions. It just has to be in the range of versions that satisfy the dependency.
If package A depends on package B ^2.1.0
then you use any version of package B that is greater or equal to 2.1
and less than 3.0
.
ember-simple-auth-token
has the following in package.json
"ember-simple-auth": "^1.6.0 || ^2.0.0 || ^3.0.0"
ember-simple-auth@3.1.0-beta.1
satisfies this dependency just like ember-simple-auth@2.1.1
does even though neither are marked as latest.
You are misunderstanding.
ember-simple-auth@3.1.0-beta.1
is not backwards compatible. It implements parts of the documented spec that are not implemented in 3.0.1. e.g. this.session.requireAuthentication
An app written for 3.1.0-beta.1 will break in really weird ways if 3.0.1 is installed instead.
ember-simple-auth-token
is calling for the latest 3.x version. Which is 3.0.1
, NOT 3.1.0-beta.1
. And Babel does not allow two addons of different versions. In this case, 3.0.1 is winning.
There is nothing wrong with ember-simple-auth-token
. The problem is here in ember-simple-auth
, using a beta version as the recommended release.
If package A depends on package B ^2.1.0 then you use any version of package B that is greater or equal to 2.1 and less than 3.0
This would only be true if package B correctly uses Semver, or the version required was in the major.minor.patch chain and was not as in this case a beta.
ember-simple-auth-token
is calling for the latest 3.x version.
It is not calling for the latest version of 3.x. It is calling for any version of 3.x that matches ^3.0.0
.
This would only be true if package B correctly uses Semver, or the version required was in the major.minor.patch chain and was not as in this case a beta.
Yes. I was not aware that 3.1.0-beta.1
did not match ^3.0.0
.
const semver = require('semver');
semver.satisfies('3.1.0-beta.1', '^1.6.0 || ^2.0.0 || ^3.0.0'); // => false
semver.satisfies('3.1.0-beta.1', '^1.6.0 || ^2.0.0 || ^3.0.0', { includePrerelease: true }); // => true
semver.satisfies('3.1.0-beta.1', '^1.6.0 || ^2.0.0 || ^3.0.0-alpha'); // => false
semver.satisfies('3.1.0-beta.1', '^1.6.0 || ^2.0.0 || ^3.0.0 || ^3.1.0-alpha'); // => true
As the recommendation in readme
is to install the beta, this inherently makes the beta a release, no? Can we not just make a 3.1.0 release?
An app written for 3.1.0-beta.1 will break in really weird ways if 3.0.1 is installed instead.
Of course it will ;) 3.1.0 introduces new APIs that do not exist in 3.0.1. That does not mean 3.1.0 is not backwards compatible.
We're aiming for a stable 3.1.0 within the next few weeks.
@BryanCrotaz I would strongly recommend to use https://github.com/salsify/ember-cli-dependency-lint to avoid these kinds of issues in the future, and if you use yarn you can use https://github.com/atlassian/yarn-deduplicate to deduplicate dependencies in your lockfile to solve the version conflicts.
:) I already use that - that's how I worked out what the problem was! I've used yarn's resolutions
to fix it. The underlying problem is that a beta version is the recommended install in the readme, and so any other addons that extend ESA will break the developer's app.
the beta is only recommended in the README of the master
branch, which corresponds to the beta release. if you look at the README for the stable version you shouldn't find such a recommendation AFAIK.
so any other addons that extend ESA will break the developer's app.
that's only true if you don't use dependency-lint or are not generally aware of the issue. and I do agree that it is an issue, but it is a more general issue, that is not just limited to ESA. even if there was a v3.1.0 release here your issue would not be resolved because the lockfile would still point to v3.0.x for existing installs.
When using ember-simple-auth-token
, you should not be explicitly installing ember-simple-auth
.
When using ember-simple-auth-token, you should not be explicitly installing ember-simple-auth.
this is starting a somewhat different discussion now, but: why not?
ember-simple-auth-token
depends on ember-simple-auth
. A compatible version of ember-simple-auth
is automatically installed when you install ember-simple-auth-token
.
sure, but that does not mean that you can't install ESA explicitly too. you only have to be careful that both versions match up. the alternative would be that ember-simple-auth-token
would declare ESA as a peer dependency instead.
@Turbo87 Yes, you are correct. You can explicitly install ESA if you want but you need to make sure to install a supported version.
I have submitted a PR to esa-token that makes ESA a peerDependency.
Please, one of you agree to change your stance. The two addons are at loggerheads, you disagree, and you both say the other is wrong. All I can do as an app developer is watch and feel sad. And fork my own solution, which is not in the ethos of the Ember community.
Please, one of you agree to change your stance. The two addons are at loggerheads, you disagree, and you both say the other is wrong
What exactly do we disagree on? I don't recall saying ESA was doing anything wrong. I said you were doing something wrong by trying to use a version of ESA that ember-simple-auth-token
does not support. I don't see a need for any changes on ESA or ember-simple-auth-token
.
Yes. I was not aware that
3.1.0-beta.1
did not match^3.0.0
.
FWIW https://jubianchi.github.io/semver-check/#/%5E3.0.0/3.1.0-beta.1%20 disagrees with that statement 🤔
@Turbo87
FWIW https://jubianchi.github.io/semver-check/#/%5E3.0.0/3.1.0-beta.1%20 disagrees with that statement
const semver = require('semver');
semver.satisfies('3.1.0-beta.1', '^1.6.0 || ^2.0.0 || ^3.0.0'); // => false
semver.satisfies('3.1.0-beta.1', '^1.6.0 || ^2.0.0 || ^3.0.0', { includePrerelease: true }); // => true
semver.satisfies('3.1.0-beta.1', '^1.6.0 || ^2.0.0 || ^3.0.0-alpha'); // => false
semver.satisfies('3.1.0-beta.1', '^1.6.0 || ^2.0.0 || ^3.0.0 || ^3.1.0-alpha'); // => true
I would assume that means semver-check
is setting includePrerelease
to true
.
Yes. I was not aware that
3.1.0-beta.1
did not match^3.0.0
.FWIW https://jubianchi.github.io/semver-check/#/%5E3.0.0/3.1.0-beta.1%20 disagrees with that statement 🤔
And yarn install
disagrees with that website.
In fact it has to - otherwise any time you put out an alpha release you'd break everyone out there.
What exactly do we disagree on? I don't recall saying ESA was doing anything wrong. I said you were doing something wrong by trying to use a version of ESA that
ember-simple-auth-token
does not support. I don't see a need for any changes on ESA orember-simple-auth-token
.
ESA is saying you should have ESA as a peerDependency, and the user should install both addons
I never said that, and I am not ESA. What I said was that it is one alternative possibility.
You are saying that it's wrong to have ESA as a peerDependency, and the user should not install both addons
I didn't say this was wrong. I said that based on the fact that ESA has historically not been a peer dependency of ember-simple-auth-token
, I'm not sure it makes sense to change it
OK, this is getting silly.
The situation is that anyone following the instructions will end up with a broken app and obscure error messages.
Between these two addons you need to sort out what you're going to do. I've submitted a PR to ember-simple-auth-token which fixes the problem. If you don't like it, come up with something else.
But don't sit there denying there's a problem and saying it doesn't make sense to change anything.
I'm off to do some work on my own code. I'll have to reimplement ESA as it's just been too much hassle coping with the upgrades.
@BryanCrotaz I will add a note to the ember-simple-auth-token
readme that says that you should let ember-simple-auth-token
install ESA or ensure that you are using a version of ESA that is compatible with ember-simple-auth-token
,
@fenichelar it looks like you're right. yarn-deduplicate
does indeed not deduplicate the ESA-token dependency and the explicitly installed ESA beta version.
I guess the only way to resolve this then is using resolutions
. I'm not sure if moving to a peer dependency would really fix anything because the version constraints would still be the same. I know that yarn only outputs warnings for broken peer dep constraints though, so that might work, but npm has roughly 4 different ways of dealing with peer deps, so no idea how the situation there works 😅
@Turbo87 What is the recommended version of ESA to use in production at this time?
I would assume the latest stable release, but I'm not the maintainer of this addon, so I can't say for sure 😅
From the docs:
However this beta is not marked as
latest
, so any other addons that require ESA won't work - Babel will use random parts of the two libraries.For example, using
ember-simple-auth-token
for JWT auth:In my case this means that
throws -
session
doesn't have any methods, only the authenticated data block. Uninstallingember-simple-auth-token
makes it work again.Please can we get a
3.1.0
release, and never recommend installation of a beta - that's guaranteed to cause these types of problems.