Possible to get a beta release that is is sync with ember-simple-auth 3.1.0-beta.0? #290

havremunken · 2021-01-17T15:52:02Z

Hi there,

I was trying to get my Ember app up to date (v3.24 as of this writing), which involves getting rid of older ways of doing things, such as using mixins. Well, in order to do that, I had to upgrade ember-simple-auth to 3.1.0-beta.0 (or .1). However, when I run my app, ember-simple-auth-token has a dependency on ESA v3.0.1 or so, which seems to override the ESA version for my whole app, so the new API does not work.

This might be a big ask since I am not familiar with this project and do not know how much is involved, but would it be possible to get a beta release of ESA-token that supports the latest betas of ESA? I realize there are potentiall any number of issues that need to be resolved to support the new API, but at least one thing is positive; Editing the node_modules/ember-simple-auth-token/package.json in my project to refer to ESA 3.1.0-beta.0 seems to work fine. I just get two deprecations, that is all.

I would be happy to contribute towards this, but I am at a very basic level with Ember so I might now have a lot to bring to the table.

Thanks for any insights!

fenichelar · 2021-01-17T19:10:22Z

@havremunken ember-simple-auth-token does not depend on any specific ember-simple-auth version. ember-simple-auth versions ^1.6.x, ^2.x, and ^3.x are all supported: https://github.com/jpadilla/ember-simple-auth-token/blob/master/package.json#L28.

You can validate this by running the following:

ember new test-app
cd test-app
ember install [email protected]
ember install ember-simple-auth-token
grep ember-simple-auth package-lock.json

The output of the grep command is:

    "ember-simple-auth": {
      "resolved": "https://registry.npmjs.org/ember-simple-auth/-/ember-simple-auth-3.1.0-beta.1.tgz",
    "ember-simple-auth-token": {
      "resolved": "https://registry.npmjs.org/ember-simple-auth-token/-/ember-simple-auth-token-5.2.0.tgz",
        "ember-simple-auth": "^1.6.0 || ^2.0.0 || ^3.0.0"
        "ember-simple-auth": {
          "resolved": "https://registry.npmjs.org/ember-simple-auth/-/ember-simple-auth-3.0.1.tgz",

You can see ember-simple-auth version 3.1.0-beta.1 is being used to satisfy the ember-simple-auth-token's dependency on ember-simple-auth.

BryanCrotaz · 2021-01-17T20:45:46Z

@havremunken ember-simple-auth-token does not depend on any specific ember-simple-auth version. ember-simple-auth versions ^1.6.x, ^2.x, and ^3.x are all supported: https://github.com/jpadilla/ember-simple-auth-token/blob/master/package.json#L28.
    "ember-simple-auth": {
      "resolved": "https://registry.npmjs.org/ember-simple-auth/-/ember-simple-auth-3.1.0-beta.1.tgz",
    "ember-simple-auth-token": {
      "resolved": "https://registry.npmjs.org/ember-simple-auth-token/-/ember-simple-auth-token-5.2.0.tgz",
        "ember-simple-auth": "^1.6.0 || ^2.0.0 || ^3.0.0"
        "ember-simple-auth": {
          "resolved": "https://registry.npmjs.org/ember-simple-auth/-/ember-simple-auth-3.0.1.tgz",
You can see ember-simple-auth version 3.1.0-beta.1 is being used to satisfy the ember-simple-auth-token's dependency on ember-simple-auth.

Actually no, it's using 3.0.1.

Part of the problem here is that ESA recommends a beta release as the release to use, but hasn't marked it as latest, so dependencies are using the wrong version.

BryanCrotaz · 2021-01-17T20:55:35Z

Workaround if using yarn:

At the end of package.json

  "resolutions": {
    "ember-simple-auth": "3.1.0-beta.1"
  }

havremunken · 2021-01-17T21:11:17Z

Yes, the problem described by @BryanCrotaz was what bit me - I will try to use the resolutions property to fix it, thanks for the suggestion!

BryanCrotaz · 2021-01-17T21:12:13Z

Thanks for your earlier comment - I've been scratching my head for hours as to why nothing was working.

BryanCrotaz · 2021-01-17T21:29:57Z

You might also find this useful if using fetch

fetch(url, authenticate(this.session));

interface EsaSession {
  data: {
    authenticated?: Record<string, string>
  }
}

export function authenticate(session: EsaSession, init?: RequestInit): RequestInit {
  if (!init) {
    init = {};
  }
  if (session.data.authenticated) {
    if (!init.headers) {
      init.headers = {};
    }
    (init.headers as Record<string, string>)["Authorization"] = `Bearer ${session.data.authenticated.access_token}`;
  }
  return init;
}

fenichelar · 2021-01-17T21:53:18Z

@BryanCrotaz

Actually no, it's using 3.0.1.

What is using 3.0.1? The sample script I provided clearly resolves to 3.1.0-beta.1. Very much by design, ember-simple-auth-token does not depend on a specific version of ember-simple-auth.

Part of the problem here is that ESA recommends a beta release as the release to use, but hasn't marked it as latest, so dependencies are using the wrong version.

This isn't really a problem. You can ember install a specific version of ember-simple-auth as I demonstrated in the shell commands.

BryanCrotaz · 2021-01-17T22:42:16Z

That's not correct. See this part of the response:

"ember-simple-auth-token": {
      "resolved": "https://registry.npmjs.org/ember-simple-auth-token/-/ember-simple-auth-token-5.2.0.tgz",
        "ember-simple-auth": "^1.6.0 || ^2.0.0 || ^3.0.0"
        "ember-simple-auth": {
          "resolved": "https://registry.npmjs.org/ember-simple-auth/-/ember-simple-auth-3.0.1.tgz",

It's resolved in the last line to 3.0.1.

This isn't really a problem

Yes, it's a big problem, thanks to the mistake made by ESA in using a beta as a release version.

You can indeed install a specific version for the containing app. But that's not how node resolution works. The app doesn't determine what the addon needs, the addon does. So npm or yarn installs both. Then only one of them can exist in the final app. Babel chooses an indeterminate one. In this case, the wrong one.

BryanCrotaz · 2021-01-17T23:03:50Z

The solution here is possibly to make ember-simple-auth a peerDependency here so that it's up to the containing app to install it, and npm/yarn will never install it twice.

fenichelar · 2021-01-17T23:14:31Z

Yes, it's a big problem, thanks to the mistake made by ESA in using a beta as a release version.

How is a beta version any different than a past version? Neither are the latest version.

The app doesn't determine what the addon needs, the addon does

How exactly does ember-simple-auth-token determine the version of ember-simple-auth that is used? Be specific. Provide the line of code in this project the references a specific version of ember-simple-auth.

By default, npm and yarn install the latest version of a package that meets the dependency at the time of install. But the app has complete control over what version is used. This is the whole reason that package-lock.json was created, so that you can force a specific version of any dependency or sub dependency and ensure all users of the package get the same version.

ember-simple-auth-tokens package.json depends on "ember-simple-auth": "^1.6.0 || ^2.0.0 || ^3.0.0" instead of just "ember-simple-auth": "^3.0.0" specifically to allow you to use old version. The old version isn't the latest version. You aren't forced to use the latest, it is just the default behavior of npm and yarn.

BryanCrotaz · 2021-01-18T00:00:55Z

You've asked for the single line of code. Then you've provided it in your answer:

"ember-simple-auth": "^1.6.0 || ^2.0.0 || ^3.0.0"

The beta version of 3.1.0 doesn't match this semver statement so it doesn't get used.

If you moved this line to a peerDependency then the problem would go away. Npm/yarn wouldn't install anything from this addon, and would just warn if the app version didn't match.

fenichelar · 2021-01-18T04:21:23Z

@BryanCrotaz

"ember-simple-auth": "^1.6.0 || ^2.0.0 || ^3.0.0"

This does not mandate a specific version of ember-simple-auth.

The beta version of 3.1.0 doesn't match this semver statement so it doesn't get used.

You are correct. I was not aware that 3.1.0-beta.1 did not match ^3.0.0.

If you moved this line to a peerDependency then the problem would go away. Npm/yarn wouldn't install anything from this addon, and would just warn if the app version didn't match.

I don't think it is as simple as just making ember-simple-auth a peerDependency. Backwards compatibility is a major goal for ember-simple-auth-token. The unit test are currently performed on 28 different combinations of ember and ember-simple-auth. I am not sure how this could easily be done if ember-simple-auth was a peerDependency.

havremunken · 2021-01-18T06:20:56Z

Thanks for the enlightening discussion, guys. It seems like this is an issue that will be best resolved by ESA finishing the beta stage and releasing a "proper" version. Until then, it would seem people could stumble into a situation like this purely by bad luck, such as I did; I wasn't familiar with the resolution done by Babel that Bryan described, but in my case, the -token dependency ended up with ESA 3.0.1 installed, and that got used for the whole app - verified in Ember Inspector.

So it is perhaps correct to say that this is not an issue that needs to be fixed by ember-simple-auth-token, but that other users might get bit by the same thing until ember-simple-auth does a non-beta release with the new APIs?

fenichelar · 2021-01-18T06:41:33Z

@havremunken The only practical thing that we could do in ember-simple-auth-token is to add support for the prereleases of 3.1.0 by changing the dependency from ^1.6.0 || ^2.0.0 || ^3.0.0 to ^1.6.0 || ^2.0.0 || ^3.0.0 || ^3.1.0-alpha or similar. We would only do this if testing scenarios were added, which means 8 new scenarios. I don't think this is something I want to do. It already takes 40+ minutes for CI to run through all the unit tests. That being said, I could be convinced if ember-simple-auth does not plan on releasing 3.1.0 for a significant amount of time and the changes in 3.1.0 provide a significant benefit.

havremunken · 2021-01-18T06:49:33Z

I understand, @fenichelar. I'm not the one with the knowledge to come up with the winning arguments - all I can say that what prompted me to get into this adventure was Octane'ifying my app, which means getting rid of mixins (to be honest, in order to try to get to zero on the linting errors etc.) - which in turn means going with the ember-simple-auth beta. The way I see it, there is no future where mixins make a comeback, so this will happen, but I completely understand wanting ember-simple-auth to complete that journey first! :)

fenichelar · 2021-01-18T06:53:54Z

What specifically does [email protected] have that you need?

havremunken · 2021-01-18T07:26:46Z

Instead of using mixins to authenticate a route, you do it using a requireAuthentication() method, as seen in the walkthrough:

https://github.com/simplabs/ember-simple-auth#walkthrough

I couldn't find an anchor to link to, but if you search for requireAuthentication on the page you will find it. Since this is presented as the "official", current version of the walkthrough, it is easy to see it as the beta being the official version, contributing to this issue.

Again, this is all coming from the desire to be Octane'y and remove the use of mixins from my Ember app.

fenichelar · 2021-01-18T07:37:47Z

@havremunken This is from an app using [email protected]:

import Route from '@ember/routing/route';
import { inject } from '@ember/service';

export default Route.extend({
  session: inject('session'),

  beforeModel: function() {
    if (!this.session.isAuthenticated) {
      this.transitionTo('login');
    }
  },
});

havremunken · 2021-01-18T09:01:41Z

Okey - so we have two possible workarounds - stick to old version and do it without using mixins that way, or force a resolution to the ESA beta to use the new API.

I am a newbie at Ember and very casual user of ESA so sorry for not having detailed knowledge of how this is interconnected. :)

BryanCrotaz · 2021-01-18T09:09:53Z

ESA-token works with all these versions of ESA.

The only issue is that the package.json is causing a second version of ESA to be installed, possibly other than what the app developer wanted.

Making ESA a peerDependency means that only the app's choice of version will be installed. If this is not in the range specified in the peerDependency, then the app developer gets a warning, but the app is not broken.

This is a one line change with no nasty side effects.

The docs need to be updated to
Ember install ember-simple-auth ember-simple-auth-token

BryanCrotaz · 2021-01-18T09:33:36Z

Okey - so we have two possible workarounds - stick to old version and do it without using mixins that way, or force a resolution to the ESA beta to use the new API.

You don't need to do either of these - the ESA Token code works with both old and new ESA.

havremunken · 2021-01-18T09:38:22Z

Absolutely, as a user I just need to be aware that it is easy to fall into the trap I did fall into, where the ESA version that was actually being used was another one than the one actually installed in my app.

BryanCrotaz · 2021-01-18T09:46:12Z

As a user, install https://github.com/salsify/ember-cli-dependency-lint

As an addon writer, use peerDependencies

fenichelar · 2021-01-18T10:25:01Z

@BryanCrotaz Changing ember-simple-auth to a peer dependency is more than a one line change.

When using ember-simple-auth-token, you should not be directly installing ember-simple-auth. Had you not directly installed ember-simple-auth, you would not have had this issue.

BryanCrotaz · 2021-01-18T10:34:19Z

When using ember-simple-auth-token, you should not be directly installing ember-simple-auth. Had you not directly installed ember-simple-auth, you would not have had this issue.

a) the readme does not say this
b) if that were the case, why do you say above "ember-simple-auth versions ^1.6.x, ^2.x, and ^3.x are all supported" - if the app did not install ember-simple-auth then you'd always get the latest version.

Changing ember-simple-auth to a peer dependency is more than a one line change.

ok, yes, it's a two line change. Which would fix the issue in perpetuity.

fenichelar · 2021-01-18T10:58:18Z

@BryanCrotaz

a) the readme does not say this

The ember-simple-auth=token readme doesn't say to install ember-simple-auth. The readme says what to do. It doesn't say what not to do.

b) if that were the case, why do you say above "ember-simple-auth versions ^1.6.x, ^2.x, and ^3.x are all supported" - if the app did not install ember-simple-auth then you'd always get the latest version.

By default, you always get the latest version at the time of install. Once it is installed, the specific version is pinned and doesn't change unless the user intentionally updates.

The point of backwards compatibility is not to let you use whatever version you want, but so that you are not forced to update ember-simple-auth when you update ember-simple-auth-token.

Also, [email protected] requires [email protected]. If you install the latest version of ember-simple-auth-token when you are using [email protected], it doesn't not resolve to the same version as when you are using [email protected].

ok, yes, it's a two line change.

What about the test scenarios?

Which would fix the issue in perpetuity.

And every current user of ember-simple-auth-token would now have to explicitly install ember-simple-auth in their app.

BryanCrotaz · 2021-01-18T11:07:22Z

Ok, you're unhelpable. I'll fork.

fenichelar · 2021-01-18T11:10:34Z

Ok, you're unhelpable. I'll fork.

Probably should have started with that and submitted a PR instead of demanding that someone else make a "one line change". I'm not saying it would have been accepted as it is a breaking change, but it would have been a lot better received.

BryanCrotaz · 2021-01-18T14:36:07Z

I made three related changes:

Move ESA to peerDependency, and use devDependency for tests
Use yarn instead of npm. One of your pushbacks on any additional testing was that testing is too slow. Yarn is approx 3x as fast as npm and you have a LOT of scenarios to install.
Designate it a v6 beta as you say it's a breaking change.

You can decide whether you want to make the world better or leave it broken. I've done the work, the tests pass on my machine so should work on CI as I've not changed any active code, so it's over to you.

I'll use my fork until and if you decide to merge.

fenichelar · 2021-01-18T14:36:51Z

@BryanCrotaz @havremunken I have been trying to determine the impact of switching ESA to a peer dependency. I know that this would require changes in all of my apps so there is one data point. Of the 3 apps listed as depending on ember-simple-auth-token here:

The GitHub repo has been deleted
Depends on both ember-simple-auth-token and ember-simple-auth
Depends on just ember-simple-auth-token

A sample size of two is pretty useless, but the fact that this change would cause an issue for 50% of the apps I can identify is a concern.

What benefit does switching ESA to a peer dependency really provide? Yes, right now it is not easy to use [email protected] when using npm and it would be easier if it was a peer dependency, but this is a beta version that hasn't been tested with ember-simple-auth so I'm not sure that is justification for this breaking change. And if you are using yarn, it is pretty easy to override the ESA version.

The reason I like the current design is because most users can just run the following:

ember install ember-simple-auth-token

Users who want a specific version can run the following:

ember install [email protected]
ember install ember-simple-auth-token

By switching to a peer dependency, everyone would have to run

ember install ember-simple-auth # or with a version number e.g.  ember install [email protected]
ember install ember-simple-auth-token

fenichelar · 2021-01-18T14:48:59Z

I made three related changes:

The canary tests don't work
You removed new lines from the end of at least one file
You pointed the ember dependency to an S3 url instead of a versions

Use yarn instead of npm. One of your pushbacks on any additional testing was that testing is too slow. Yarn is approx 3x as fast as npm and you have a LOT of scenarios to install.

Look at CI, it isn't any faster.

You can decide whether you want to make the world better or leave it broken. I've done the work, the tests pass on my machine so should work on CI as I've not changed any active code, so it's over to you.

I think this makes the world worse, not better.

BryanCrotaz · 2021-01-18T14:49:00Z

The reason I like the current design is because most users can just run the following:
ember install ember-simple-auth-token

In real life that's not what happens.

Read the ESA readme... install...
ember install [email protected]

Oh! I want to use OAuth with google identities, so I need JWT. Ember-simple-auth doesn't support JWT, so I need an authenticator. OK, I'll write one. Oh there's a lot to this - I wonder if someone's already done it. Oh! They have!
ember install ember-simple-auth-token

SH*T! My app has stopped working. Hang on, this.session doesn't resolve to the ESA session service any more. HUH??? 8 hours later, discover this Github thread.

I will bet you that 99% of your users found ESA first, and this addon second.

future peerDependency real world

ember install ember-simple-auth-token
ember serve
warning - peerDependency ember-simple-auth>1.6.0 not found - please install it
ember install ember-simple-auth
ember serve

You can even add a default blueprint that runs on ember install that adds ember-simple-auth if you don't have it. https://cli.emberjs.com/release/writing-addons/addon-blueprints/

fenichelar · 2021-01-18T14:54:02Z

In real life that's not what happens.

I showed evidence of it happening in the wild so could you please provide evidence of this claim.

The issue you described only applies if you use a beta version of ESA. I'm a lot more concerned about users not use beta versions than I am about users that are using beta versions.

havremunken · 2021-01-18T16:17:51Z

@fenichelar I am not sure the beta version argument is the killer argument here. For instance, what happened to me is I was working on modernizing my ancient (well, 3.18 or so) Ember app. Reading up on ESA, I noticed that they were moving to new, mixin-less APIs.

So I opened a new branch in git, moved to it, and started upgrading. A day later and here we are.

Now, I agree it wasn't clear that ESA are consistently talking about the beta as being the latest version, but based on what we now know, I would have kept this branch in development, and only when they release "in a few weeks", it would have made it into production.

However, without working around the dependency issue, I could not have even started this work until ESA were to release. And this is not an experimental change they are doing. All their documentation etc. makes it pretty clear that this IS the future, this is going to be the way to do it.

So developer productivity wise, I'm not convinced that ESA being in beta is the argument for nixing this.

And I do agree with @BryanCrotaz that from my perspective, ESA is the addon I add to my app. Then I add plugins to that to add functionality. That is the case with similar software like ember-cli-deploy, so it makes sense for me here as well. Therefore, ESA is not just a dependency of ember-simple-auth-token that happens to tag along, it is the "main course" and the reason for all of this to be included in the first place. Like pointed out before, I choose ESA plugins based on their fit for my project, I don't start with the plugin/extension and then just happen to install ESA by happy accident.

I concede that this whole situation is probably caused by ESA beta being perceived as production ready, but for developer ergonomys sake, I see no reason to let -token be the "owner" of ESA in my project. My app uses ESA, so I want to control the version. Therefore, the assertion that one should only get ESA as a dependency of ESA-token doesn't hold water IMO.

fenichelar · 2021-01-18T16:46:16Z

Now, I agree it wasn't clear that ESA are consistently talking about the beta as being the latest version, but based on what we now know, I would have kept this branch in development, and only when they release "in a few weeks", it would have made it into production.

What? You are reading the readme of a pre-release and for some reason acting like this is the recommended version to use. ESA intentionally made this a prerelease.

What if someone updated the readme on the master branch but didn't release anything. Would you say that they are telling you to use the master branch instead of the released version?

I concede that this whole situation is probably caused by ESA beta being perceived as production ready, but for developer ergonomys sake, I see no reason to let -token be the "owner" of ESA in my project. My app uses ESA, so I want to control the version. Therefore, the assertion that one should only get ESA as a dependency of ESA-token doesn't hold water IMO.

I have very clearly demonstrated that you can manage ESA yourself with the current design.

Like pointed out before, I choose ESA plugins based on their fit for my project, I don't start with the plugin/extension and then just happen to install ESA by happy accident.

And that is fine but others don't necessarily do that. In fact, I have provided an example where that is not the case. You are asking for a breaking change because you perceived a prerelease version of ESA to be production ready.

I concede that this whole situation is probably caused by ESA beta being perceived as production ready, but for developer ergonomys sake

Why are you perceiving this as production ready? It was put out as a prerelease... That pretty clearly tells you it isn't production ready.

fenichelar · 2021-01-18T17:17:20Z

Also, when using npm with the peer dependency, there is no warning if you use an incompatible version of ember-simple-auth. See proof below.

So no matter what, if you want to install ember-simple-auth manually, you need to make sure you that you are installing a version that is compatible with ember-simple-auth-token.

If we switch to a peer dependency and then drop support for [email protected], it will silently fail for everyone using [email protected] with npm.

Given using peer dependencies just shifts the potential issue somewhere else, I don't think it makes sense. I will update the readme to reflect the fact that you should make sure to only install a version of ember-simple-auth that is supported by ember-simple-auth-token.

ember new test-app
cd test-app
ember install [email protected]
ember install "https://github.com/BryanCrotaz/ember-simple-auth-token#fix-dependency-on-ESA"
npm run build

Full output:

alec@computer ~/Downloads $ ember new test-app                                                                                                                     [ruby-2.6.3p62]
installing app
Ember CLI v3.16.0

✨  Creating a new Ember app in /Users/alec/Downloads/test-app:
  create .editorconfig
  create .ember-cli
  create .eslintignore
  create .eslintrc.js
  create .template-lintrc.js
  create .travis.yml
  create .watchmanconfig
  create README.md
  create app/app.js
  create app/components/.gitkeep
  create app/controllers/.gitkeep
  create app/helpers/.gitkeep
  create app/index.html
  create app/models/.gitkeep
  create app/router.js
  create app/routes/.gitkeep
  create app/styles/app.css
  create app/templates/application.hbs
  create config/environment.js
  create config/optional-features.json
  create config/targets.js
  create ember-cli-build.js
  create .gitignore
  create package.json
  create public/robots.txt
  create testem.js
  create tests/helpers/.gitkeep
  create tests/index.html
  create tests/integration/.gitkeep
  create tests/test-helper.js
  create tests/unit/.gitkeep
  create vendor/.gitkeep

🚧  Installing packages... This might take a couple of minutes.
npm: Installed dependencies

🎥  Initializing git repository.
Git: successfully initialized.

🎉  Successfully created project test-app.
👉   Get started by typing:

  $ cd test-app
  $ npm start

Happy coding!
alec@computer ~/Downloads $ cd test-app                                                                                                                            [ruby-2.6.3p62]
alec@computer ~/Downloads/test-app (master)$ ember install [email protected]                                                                          [ruby-2.6.3p62]

🚧  Installing packages... This might take a couple of minutes.
npm: Installed [email protected]
Installed addon package.
alec@computer ~/Downloads/test-app (master●)$ ember install "https://github.com/BryanCrotaz/ember-simple-auth-token#fix-dependency-on-ESA"                         [ruby-2.6.3p62]

🚧  Installing packages... This might take a couple of minutes.
npm: Installed https://github.com/BryanCrotaz/ember-simple-auth-token#fix-dependency-on-ESA
WARNING: Could not figure out blueprint name from: "https://github.com/BryanCrotaz/ember-simple-auth-token#fix-dependency-on-ESA". Please install the addon blueprint via "ember generate <addon-name>" if necessary.
Installed addon package.
alec@computer ~/Downloads/test-app (master●)$ npm run build                                                                                                        [ruby-2.6.3p62]

> [email protected] build /Users/alec/Downloads/test-app
> ember build --environment=production

⠋ BuildingWARNING: Option "nodeWorker" is deprecated since [email protected]. Please use "workerType" instead.
Environment: production
cleaning up...
Built project successfully. Stored in "dist/".
File sizes:
 - dist/assets/test-app-d41d8cd98f00b204e9800998ecf8427e.css: 0 B
 - dist/assets/test-app-e4896a28888c232efb8528927719edb2.js: 12.42 KB (2.54 KB gzipped)
 - dist/assets/vendor-d41d8cd98f00b204e9800998ecf8427e.css: 0 B
 - dist/assets/vendor-e45296fbf227d984c72dcf3c305b6e18.js: 746.54 KB (187.89 KB gzipped)

BryanCrotaz · 2021-01-18T17:27:43Z

Of course, you would never have that problem, because that would be a breaking change, and therefore a new major version, so no one would have it foisted on an old app..

…

On Mon, 18 Jan 2021 at 17:17, Alec Fenichel ***@***.***> wrote: Also, when using npm with the peer dependency, there is no warning if you use an incompatible version of ember-simple-auth. See proof below. So no matter what, if you want to install ember-simple-auth manually, you need to make sure you that you are installing a version that is compatible with ember-simple-auth-token. If we switch to a peer dependency and then drop support for ***@***.***, it will silently fail for everyone using ***@***.*** with npm. Given using peer dependencies just shifts the potential issue somewhere else, I don't think it makes sense. I will update the readme to reflect the fact that you should make sure to only install a version of ember-simple-auth that is supported by ember-simple-auth-token. ember new test-app cd test-app ember install ***@***.*** ember install "https://github.com/BryanCrotaz/ember-simple-auth-token#fix-dependency-on-ESA" npm run build Full output: ***@***.*** ~/Downloads $ ember new test-app [ruby-2.6.3p62] installing app Ember CLI v3.16.0 ✨ Creating a new Ember app in /Users/alec/Downloads/test-app: create .editorconfig create .ember-cli create .eslintignore create .eslintrc.js create .template-lintrc.js create .travis.yml create .watchmanconfig create README.md create app/app.js create app/components/.gitkeep create app/controllers/.gitkeep create app/helpers/.gitkeep create app/index.html create app/models/.gitkeep create app/router.js create app/routes/.gitkeep create app/styles/app.css create app/templates/application.hbs create config/environment.js create config/optional-features.json create config/targets.js create ember-cli-build.js create .gitignore create package.json create public/robots.txt create testem.js create tests/helpers/.gitkeep create tests/index.html create tests/integration/.gitkeep create tests/test-helper.js create tests/unit/.gitkeep create vendor/.gitkeep 🚧 Installing packages... This might take a couple of minutes. npm: Installed dependencies 🎥 Initializing git repository. Git: successfully initialized. 🎉 Successfully created project test-app. 👉 Get started by typing: $ cd test-app $ npm start Happy coding! ***@***.*** ~/Downloads $ cd test-app [ruby-2.6.3p62] ***@***.*** ~/Downloads/test-app (master)$ ember install ***@***.*** [ruby-2.6.3p62] 🚧 Installing packages... This might take a couple of minutes. npm: Installed ***@***.*** Installed addon package. ***@***.*** ~/Downloads/test-app (master●)$ ember install "https://github.com/BryanCrotaz/ember-simple-auth-token#fix-dependency-on-ESA" [ruby-2.6.3p62] 🚧 Installing packages... This might take a couple of minutes. npm: Installed https://github.com/BryanCrotaz/ember-simple-auth-token#fix-dependency-on-ESA WARNING: Could not figure out blueprint name from: "https://github.com/BryanCrotaz/ember-simple-auth-token#fix-dependency-on-ESA". Please install the addon blueprint via "ember generate <addon-name>" if necessary. Installed addon package. ***@***.*** ~/Downloads/test-app (master●)$ npm run build [ruby-2.6.3p62] > ***@***.*** build /Users/alec/Downloads/test-app > ember build --environment=production ⠋ BuildingWARNING: Option "nodeWorker" is deprecated since ***@***.*** Please use "workerType" instead. Environment: production cleaning up... Built project successfully. Stored in "dist/". File sizes: - dist/assets/test-app-d41d8cd98f00b204e9800998ecf8427e.css: 0 B - dist/assets/test-app-e4896a28888c232efb8528927719edb2.js: 12.42 KB (2.54 KB gzipped) - dist/assets/vendor-d41d8cd98f00b204e9800998ecf8427e.css: 0 B - dist/assets/vendor-e45296fbf227d984c72dcf3c305b6e18.js: 746.54 KB (187.89 KB gzipped) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#290 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAPMKCZMDZWKPOJEFSQGWXTS2RULBANCNFSM4WGGO6NA> .

-- Bryan Crotaz Managing Director Silver Curve 0758 404 8886

fenichelar · 2021-01-18T17:32:57Z

Of course, you would never have that problem, because that would be a breaking change, and therefore a new major version, so no one would have it foisted on an old app..

Why are you assuming that they are going to read what versions of ember-simple-auth are supported when they update to a new version of ember-simple-auth-token? You didn't read it... That is why you are having this problem.

In years of operating ember-simple-auth-token, this is the first time this has come up. And the only reason you are facing this issue is because you are attempting to use a prerelease version of ember-simple-auth.

BryanCrotaz · 2021-01-18T17:33:19Z

I suggest you take thought leadership from the ember-cli-deploy methodology - it's well established and works really well.

BryanCrotaz · 2021-01-18T17:35:13Z

Anyway, I'm off to write my own addon. @havremunken I'll let you know when it's ready.

fenichelar · 2021-01-18T17:39:01Z

@BryanCrotaz You opened an issue with ESA asking for a release and they said:

We're aiming for a stable 3.1.0 within the next few weeks.

Are you really saying that we need to make breaking changes to ember-simple-auth-token so that it is easier for you to use use unstable version of ember-simple-auth?

You think it is right to make everyone who wants to use ember-simple-auth-token manually install ember-simple-auth so that you don't have to add a yarn resolutions entry to use an unstable version of ESA?

BryanCrotaz · 2021-01-18T17:42:44Z

No, I'm saying your methodology is fundamentally borked such that a developer CANNOT use the latest beta of ESA, not just this one. In this case, this beta has been around for about 6 months, and it is the first version to properly support Octane with ES6 classes. There is no option for any Octane app but to use the beta. I've been using it in production for ages. Next time ESA fixes a major bug / missing feature with a beta, all your users will be stuffed again because you can't see past your own usage pattern.

…

On Mon, 18 Jan 2021 at 17:39, Alec Fenichel ***@***.***> wrote: @BryanCrotaz <https://github.com/BryanCrotaz> You opened an issue with ESA asking for a release and they said: We're aiming for a stable 3.1.0 within the next few weeks. Are you really saying that we need to make breaking changes to ember-simple-auth-token so that it is easier for you to use use unstable version of ember-simple-auth? You think it is right to make everyone who wants to use ember-simple-auth-token manually install ember-simple-auth so that you don't have to add a yarn resolutions entry to use an unstable version of ESA? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#290 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAPMKC55KTDUY2RIC3EB5RLS2RW4LANCNFSM4WGGO6NA> .

-- Bryan Crotaz Managing Director Silver Curve 0758 404 8886

fenichelar · 2021-01-18T17:42:59Z

@BryanCrotaz

Anyway, I'm off to write my own addon

Are you also planning on writing your own version of https://github.com/auth0-community/ember-simple-auth-auth0?

BryanCrotaz · 2021-01-18T17:47:58Z

Well I don't use auth0, but it looks like they've made the same mistake as you, so if I was using it, then I'd either have to hope they can see the problem they've caused and work with me or accept a PR, or I'd have to write my own.

I can't just call my users and say "sorry, we can't ship all these lovely tasty Octane features because an addon can't play nicely, but don't worry, they've said they hope to ship a release 'soon', although the last commit was 5 months ago, so maybe see you in July..."

fenichelar · 2021-01-18T17:51:53Z

No, I'm saying your methodology is fundamentally borked such that a developer CANNOT use the latest beta of ESA, not just this one.

ember-simple-auth-token can't use the beta because it isn't supported! Your peer dependency change doesn't change the fact that it isn't supported. It just makes it easier for you to ignore the fact that it isn't supported.

In this case, this beta has been around for about 6 months, and it is the first version to properly support Octane with ES6 classes. There is no option for any Octane app but to use the beta. I've been using it in production for ages.

If you are simply interested in using the ESA beta then ask for ember-simple-auth-token to support the beta. If it is so worthwhile, then we can do it. Maybe we will make a beta release of ember-simple-auth-token the supports ESA and then we won't have to make a breaking change to drop support of the ESA beta.

Next time ESA fixes a major bug / missing feature with a beta, all your users will be stuffed again because you can't see past your own usage pattern

No, next time they make a change all my user's won't be using unsupported version of ESA and opening issues here for a prerelease with bugs.

I can't just call my users and say "sorry, we can't ship all these lovely tasty Octane features because an addon can't play nicely, but don't worry, they've said they hope to ship a release 'soon', although the last commit was 5 months ago, so maybe see you in July..."

If you want ember-simple-auth-token to support the beta, ask for that. Not suggest a breaking change that impacts everyone.

fenichelar · 2021-01-18T17:53:20Z

@BryanCrotaz @havremunken I will make a beta version of ember-simple-auth-token that supports the latest ESA beta. This change won't impact anyone else so it isn't a big deal.

fenichelar · 2021-01-18T18:10:26Z

Once #295 is merged I will create a new branch based on this and add support for [email protected]. The beta ESA version will be added to the dependencies as a supported version and added to the tests.

havremunken · 2021-01-18T18:14:11Z

What? You are reading the readme of a pre-release and for some reason acting like this is the recommended version to use. ESA intentionally made this a prerelease.

What? THIS is what you're getting from a post that you quoted out of context saying SPECIFICALLY that I created a branch for the ESA beta that wouldn't go into production?

I've stated that I'm new at Ember, that I don't know ESA-token (or ESA) in much detail, and specified that I came to the ESA beta from their docs site where it was presented as this is their currently stable API. I'm sorry that you seem to take this github issue as a personal insult, it was never meant as such. I just want to be able to go on with my development, and the inclusion of ESA as a dependency acted as a showstopper for that. I don't want you to change everything about this project.

I see now that the magic words were "Can we have a beta version of ESA-token please?" - I'm sorry I didn't simply ask for that, as stated above I didn't have the knowledge of how this all fits together.

What if someone updated the readme on the master branch but didn't release anything. Would you say that they are telling you to use the master branch instead of the released version?

What if someone published the docs on ESA beta as if that is the production version?

And that is fine but others don't necessarily do that. In fact, I have provided an example where that is not the case. You are asking for a breaking change because you perceived a prerelease version of ESA to be production ready.

No, I'm not. I was asking how to go about taking care of this situation, believing it to be something other than it was. You chose to interpret it as a personal attack. All I want is to be able to get my app in line with Octane, and this stopped me from doing that. If you reread the whole thread I think you will find that noone has demanded that you break the whole project for us.

Why are you perceiving this as production ready? It was put out as a prerelease... That pretty clearly tells you it isn't production ready.

Again, didn't you read my whole post, or are you just ignoring it on purpose? I SPECIFICALLY stated I created a separate branch for starting ESA modernization work. To do development until it is ready.

This project is a great piece of software, but it could do without the attitude and hostility to new users.

fenichelar · 2021-01-18T18:38:28Z

@havremunken I apologize for the handling of this issue. I did not see your comments as a personal attack. I did not intend to be hostile towards you. I want to help you fix your issue.

This is no excuse, but my frustration has been due to @BryanCrotaz's attitude:

This is a one line change with no nasty side effects.

As an addon writer, use peerDependencies

ok, yes, it's a two line change. Which would fix the issue in perpetuity.

Ok, you're unhelpable. I'll fork.

You can decide whether you want to make the world better or leave it broken. I've done the work, the tests pass on my machine so should work on CI as I've not changed any active code, so it's over to you.

I suggest you take thought leadership from the ember-cli-deploy methodology - it's well established and works really well.

Anyway, I'm off to write my own addon. @havremunken I'll let you know when it's ready.

No, I'm saying your methodology is fundamentally borked such that a developer CANNOT use the latest beta of ESA, not just this one

Well I don't use auth0, but it looks like they've made the same mistake as you, so if I was using it, then I'd either have to hope they can see the problem they've caused and work with me or accept a PR, or I'd have to write my own.

Making a breaking change needs to be carefully considered. If there is a critical security issue or bug, I try to patch as many versions as possible. Each major version means more maintenance. It also means users have to manually update to get new features added after the major version bump. Breaking changes should be avoided when possible.

havremunken · 2021-01-18T18:50:47Z

Look, I'm sorry too - there was no reason to get angry from my side either. It is just a result of frustration. Like @BryanCrotaz I had spent quite a bit of time trying to understand and fix the problem, and things escalate as they can do when you're talking through text to someone you don't know and has a different perspective than you do. I'm sure you're a good guy - you wouldn't do the thankless job of maintaining open source software otherwise. I'm pretty sure we can get to a good place here, and if nothing else, this little exchange has probably left the three of us with a better understanding of where we want to be.

Friends again? Beers for all involved. :)

fenichelar · 2021-01-18T18:53:54Z

I don't want my last comment to come across as that the only reason peer dependencies is a problem is because it is a breaking change. Even if we made ESA a peer dependency, ember-simple-auth-token won't technically support ESA betas. So if the goal is to support beta installs, this doesn't really fix the problem. Also, peer dependency violations don't generate an error or warning with npm, so either way if a user installs an incompatible version of ESA, you can get silent bugs. This is why I am looking for a better way to address this issue.

fenichelar · 2021-01-18T18:54:22Z

🍺🍺🍺

fenichelar · 2021-01-18T19:22:37Z

I wonder if this addon could check the ESA version similar to what https://github.com/salsify/ember-cli-dependency-lint
and https://github.com/dgeb/ember-cli-addon-guard do. This would allow us to alert when an unsupported version of ESA is being used.

fenichelar · 2021-01-18T21:46:28Z

@havremunken You can do the following: ember install "https://github.com/fenichelar/ember-simple-auth-token#esa-3.1.0-beta". I have run the full set of test scenarios and they all pass.

Proof of concept:

ember new test-app
cd test-app
ember install [email protected]
ember install "https://github.com/fenichelar/ember-simple-auth-token#esa-3.1.0-beta"

grep ember-simple-auth package.json

    "ember-simple-auth": "^3.1.0-beta.1",
    "ember-simple-auth-token": "git+https://github.com/fenichelar/ember-simple-auth-token.git#esa-3.1.0-beta",

grep ember-simple-auth package-lock.json

    "ember-simple-auth": {
      "resolved": "https://registry.npmjs.org/ember-simple-auth/-/ember-simple-auth-3.1.0-beta.1.tgz",
    "ember-simple-auth-token": {
      "version": "git+https://github.com/fenichelar/ember-simple-auth-token.git#beb09341a9f1b761d6281a3820e859643ba85ac0",
      "from": "git+https://github.com/fenichelar/ember-simple-auth-token.git#esa-3.1.0-beta",
        "ember-simple-auth": "^1.6.0 || ^2.0.0 || ^3.0.0 || ^3.1.0-beta.1"

havremunken · 2021-01-19T06:25:23Z

That is awesome, thank you! Will test it as soon as I get through some annoying stuff on another branch. :)

fenichelar self-assigned this Jan 17, 2021

BryanCrotaz mentioned this issue Jan 17, 2021

Recommendation to install beta breaks ecosystem mainmatter/ember-simple-auth#2260

Open

This was referenced Jan 18, 2021

Move ESA to peerDependency and update tests. #293

Closed

Add install note to readme #295

Merged

fenichelar closed this as completed in #295 Feb 1, 2021

Possible to get a beta release that is is sync with ember-simple-auth 3.1.0-beta.0? #290

Possible to get a beta release that is is sync with ember-simple-auth 3.1.0-beta.0? #290

Comments

havremunken commented Jan 17, 2021

fenichelar commented Jan 17, 2021

BryanCrotaz commented Jan 17, 2021 • edited Loading

BryanCrotaz commented Jan 17, 2021

havremunken commented Jan 17, 2021

BryanCrotaz commented Jan 17, 2021

BryanCrotaz commented Jan 17, 2021 • edited Loading

fenichelar commented Jan 17, 2021

BryanCrotaz commented Jan 17, 2021 • edited Loading

BryanCrotaz commented Jan 17, 2021

fenichelar commented Jan 17, 2021

BryanCrotaz commented Jan 18, 2021 • edited Loading

fenichelar commented Jan 18, 2021

havremunken commented Jan 18, 2021

fenichelar commented Jan 18, 2021

havremunken commented Jan 18, 2021

fenichelar commented Jan 18, 2021

havremunken commented Jan 18, 2021

fenichelar commented Jan 18, 2021

havremunken commented Jan 18, 2021

BryanCrotaz commented Jan 18, 2021

BryanCrotaz commented Jan 18, 2021

havremunken commented Jan 18, 2021

BryanCrotaz commented Jan 18, 2021

fenichelar commented Jan 18, 2021

BryanCrotaz commented Jan 18, 2021

fenichelar commented Jan 18, 2021 • edited Loading

BryanCrotaz commented Jan 18, 2021

fenichelar commented Jan 18, 2021

BryanCrotaz commented Jan 18, 2021

fenichelar commented Jan 18, 2021

fenichelar commented Jan 18, 2021

BryanCrotaz commented Jan 18, 2021 • edited Loading

future peerDependency real world

fenichelar commented Jan 18, 2021

havremunken commented Jan 18, 2021

fenichelar commented Jan 18, 2021

fenichelar commented Jan 18, 2021

BryanCrotaz commented Jan 18, 2021 via email

fenichelar commented Jan 18, 2021

BryanCrotaz commented Jan 18, 2021

BryanCrotaz commented Jan 18, 2021

fenichelar commented Jan 18, 2021

BryanCrotaz commented Jan 18, 2021 via email

fenichelar commented Jan 18, 2021

BryanCrotaz commented Jan 18, 2021

fenichelar commented Jan 18, 2021 • edited Loading

fenichelar commented Jan 18, 2021

fenichelar commented Jan 18, 2021

havremunken commented Jan 18, 2021

fenichelar commented Jan 18, 2021

havremunken commented Jan 18, 2021

fenichelar commented Jan 18, 2021 • edited Loading

fenichelar commented Jan 18, 2021 • edited Loading

fenichelar commented Jan 18, 2021

fenichelar commented Jan 18, 2021

havremunken commented Jan 19, 2021

BryanCrotaz commented Jan 17, 2021 •

edited

Loading

BryanCrotaz commented Jan 17, 2021 •

edited

Loading

BryanCrotaz commented Jan 17, 2021 •

edited

Loading

BryanCrotaz commented Jan 18, 2021 •

edited

Loading

fenichelar commented Jan 18, 2021 •

edited

Loading

BryanCrotaz commented Jan 18, 2021 •

edited

Loading

fenichelar commented Jan 18, 2021 •

edited

Loading

fenichelar commented Jan 18, 2021 •

edited

Loading

fenichelar commented Jan 18, 2021 •

edited

Loading