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

Widen ember-get-config dependency requirement (include v0.4 and v0.5) #2242

Conversation

rahulk94
Copy link
Contributor

This PR bumps the minimum required version of ember-get-config to 0.3.0. This is required as 0.2.4 uses ember-cli-babel@6 which throws deprecation warnings when consumers of this project use Ember 3.28 (didn't drill into which specific version adds this deprecation warning).

This is the deprecation warning this PR fixes

WARNING: [DEPRECATION] [DEPRECATION] Usage of the Ember Global is deprecated. You should import the Ember module or the specific API instead.

See https://deprecations.emberjs.com/v3.x/#toc_ember-global for details.

Usages of the Ember Global may be caused by an outdated ember-cli-babel dependency. The following steps may help:

* Upgrade the following addons to the latest version:
  * ember-cli-mirage
  ...
### Important ###

In order to avoid repeatedly showing the same deprecation messages, no further deprecation messages will be shown for usages of the Ember Global until ember-cli-babel is upgraded to v7.26.6 or above.

To see all instances of this deprecation message, set the `EMBER_GLOBAL_DEPRECATIONS` environment variable to "all", e.g. `EMBER_GLOBAL_DEPRECATIONS=all ember test`.

### Details ###

Prior to v7.26.6, ember-cli-babel sometimes transpiled imports into the equivalent Ember Global API, potentially triggering this deprecation message indirectly, even when you did not observe these deprecated usages in your code.

The following outdated versions are found in your project:

  * [email protected]
    * Depends on ember-cli-babel@^6.3.0
    * Added by [email protected]

Note: Addons marked as "Dormant" does not appear to have any JavaScript files. Therefore, even if they are using an old version ember-cli-babel, they are unlikely to be the culprit of this deprecation and can likely be ignored.

There was some related conversation over on #2227 (comment) to this as well.

Functionally nothing should really change here besides ember-cli-mirage requiring a slightly newer version of the dependency but that was already an allowed one so not a biggie imo.

@SergeAstapov
Copy link
Collaborator

@rahulk94 the actual version should be determined by the consumer in the consuming app and restricting only to higher version of the addon makes it much harder to upgrade all the addons should any other addon use ember-get-config 0.2.*

In fact, it's better to loosen such restrictions to broaden support ranges and not force uses to resort to resolutions.

@cah-brian-gantzler
Copy link
Collaborator

cah-brian-gantzler commented Nov 30, 2021

So are you saying that what is was before "^0.2.4 || ^0.3.0" is what it should be? Then the consumer add would add "ember-get-config": "^0.3.0", to its package.json?

I think the key might have been the update to the yarn.lock only so you got 0.3.0 if you did NOT specify ember-get-config in the consuming package.json

@rahulk94
Copy link
Contributor Author

Part of the problem here tbh is that ember-get-config hasn't had a 1.x release so the ^ doesn't end up working properly 🥲 (its coming after it gets embroider support mansona/ember-get-config#21).

In fact, it's better to loosen such restrictions to broaden support ranges and not force uses to resort to resolutions.

I would think that tightening restrictions makes sense when resolving deprecations though as you want to force people to use newer stuff? Or were you getting more at the fact that this should instead be now ^0.3.0 || ^0.4.0 || ^0.5.0?

@SergeAstapov
Copy link
Collaborator

SergeAstapov commented Nov 30, 2021

@rahulk94 I personally think this should be "^0.2.4 || ^0.3.0 || ^0.4.0 || ^0.5.0" or for brevity "0.2.4 - 0.5.0 just to give the users opportunity to specify any valid version.

As an example, we are stuck on "ember-get-config": "^0.2.4" due to another addon depending on ^0.2.4 and we need some major work to do for major version bump of that dependency so just bear for the time being with ^0.2.4 and all the noise of deprecations.

but in case any other dependency would restrict to ^0.3.0, it would make things quite difficult as we would need to upgrade bunch of things at the same time (just a note, we use https://github.com/salsify/ember-cli-dependency-lint in our CI).

E.g. if ember-cli-mirage had "ember-get-config": "0.2.4 - 0.5.0"and in your app you have"ember-get-config": "0.2.4 - 0.5.0"` - then you would simply get the latest version.

@rahulk94
Copy link
Contributor Author

"0.2.4 - 0.5.0" works for me.

@SergeAstapov or @cah-briangantzler do either of you know how to trigger the CI builds on this project? I can bump and test it locally / integration test with an app but yea I'm not sure how the Travis builds are done here (or do we need to move to Actions since I heard Travis ain't free anymore or have significantly cut back on their free tier?)

@cah-brian-gantzler
Copy link
Collaborator

The travis builds no longer work. We are working on a PR to get the tests running using github actions.

MirageJS is currently at

"miragejs": "^0.1.31"

assuming we should be changing that to

"miragejs": "0.1.31 - 0.1.42"

as well

@rahulk94
Copy link
Contributor Author

rahulk94 commented Nov 30, 2021

Dam it Travis 😢 I thought that was the case. Sweet well I see you're doing some stuff in other PRs so I'll park this for the moment but ping me and I can rebase this work on to master after the GitHub actions work has been done @cah-briangantzler (or any sooner if you think it makes sense). Btw ty for taking ownership of this project (or well trying to 😁 ), very much appreciated.

@cah-brian-gantzler
Copy link
Collaborator

You going to be updating this PR to "0.2.4 - 0.5.0"

As of yet I still dont have the authority to publish (It was all automated via travis 😞 ), so this still may set a while.

@rahulk94
Copy link
Contributor Author

rahulk94 commented Nov 30, 2021

You going to be updating this PR to "0.2.4 - 0.5.0"

Yep I'll update it to this

Edit: Done now

@cah-brian-gantzler cah-brian-gantzler merged commit 63efdac into miragejs:master Dec 1, 2021
@SergeAstapov SergeAstapov added the Dependencies Pull requests that update a dependency file label Dec 2, 2021
@cah-brian-gantzler
Copy link
Collaborator

Is this a breaking change?

I have a friend with an app in 2.18 using mirage. If he is using ember-get-config as a transitive dependency, upgrading could get him 0.5.0 which states is dropped support for < 3.4 https://github.com/mansona/ember-get-config/releases/tag/v0.5.0 it would break his build. The fix is for them to add ember-get-config 0.4.0 or less to their package.json.

But since it requires more than just an update to ember-cli-mirage, does this merged PR represent a breaking change when we do the next release?

@SergeAstapov
Copy link
Collaborator

@cah-briangantzler it should not. the package manager should be able to properly resolve to the version of ember-get-config that already exists in the tree

@SergeAstapov SergeAstapov changed the title chore/upgrade-ember-get-config-dependency-for-deprecation-fix Widen ember-get-config dependency requirement Dec 7, 2021
@SergeAstapov SergeAstapov changed the title Widen ember-get-config dependency requirement Widen ember-get-config dependency requirement (include v0.4 and v0.5) Dec 7, 2021
@SergeAstapov SergeAstapov added Feature / Enhancement and removed Dependencies Pull requests that update a dependency file labels Jan 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants