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

Deprecate miragejs re-exports from ember-cli-mirage/* modules #2256

Merged
merged 3 commits into from
Jan 10, 2022

Conversation

SergeAstapov
Copy link
Collaborator

This is continuation for #2244.

As discussed with @cah-briangantzler, this is necessary for the next major release where ember-cli-mirage should become thiner Ember.sj glue to bring miragejs to the Ember world.

Few more re-exports needs to be updated/deprecated but you should be able to grasp the direction this is heading.

@cah-brian-gantzler
Copy link
Collaborator

Yea, Most these files will be able to be deleted in the breaking release

@cah-brian-gantzler
Copy link
Collaborator

I thought you had a PR merged that added the auto-import 2.x to the tests? Or is that still a work in progress

@SergeAstapov
Copy link
Collaborator Author

I thought you had a PR merged that added the auto-import 2.x to the tests? Or is that still a work in progress

I think you refer to https://github.com/miragejs/ember-cli-mirage/pull/2253/files where I updated ember-try scenarios as I believe ember-auto-import v2 is a breaking change and we'll do it once we start working on v3 of this addon

@cah-brian-gantzler
Copy link
Collaborator

Yes thats what I was referring to. But since it was merged I wondered why this failed the ember 4.x tests. I hadnt looked, thought it was auto import, now it looks like is Named outlets were removed in Ember 4.0. Not sure where that is being used in the tests. Prob the docs and updating addon docs will get past that. Perhaps in addon docs and when they are updated that might go away.

@SergeAstapov
Copy link
Collaborator Author

SergeAstapov commented Dec 9, 2021

@cah-briangantzler I've outlined the (IMO) safe plan for addon-docs upgrade in this comment, which is IMO orthogonal to the changes in this PR but still should bring us closer to the new [major] release and green CI.

@SergeAstapov SergeAstapov force-pushed the deprecate-reexports branch 2 times, most recently from f388129 to 38efaeb Compare December 9, 2021 06:21
@cah-brian-gantzler
Copy link
Collaborator

Since you have extracted a lot of the code to some methods, you might want to consider (in the future) creating an addon that supplies this re-export deprecating. I googled for a long time and didnt find any code to do it. So an addon that would provide this for others when they need it would be nice.

@SergeAstapov SergeAstapov force-pushed the deprecate-reexports branch 2 times, most recently from 47ec3c3 to 3fa675d Compare December 14, 2021 00:41
@SergeAstapov SergeAstapov marked this pull request as ready for review December 14, 2021 01:37
@SergeAstapov SergeAstapov changed the title Deprecate miragejs re-exports from ember-cli-mirage/* modules Deprecate miragejs re-exports from ember-cli-mirage/* modules Dec 23, 2021
@cah-brian-gantzler
Copy link
Collaborator

I think this is fantastic work and sad that it will all go away when the deprecations are removed. I think that some of the code could be put in an addon to be used other addons that also need to deprecate their own exports.

@SergeAstapov
Copy link
Collaborator Author

I think this is fantastic work and sad that it will all go away when the deprecations are removed. I think that some of the code could be put in an addon to be used other addons that also need to deprecate their own exports.

Thanks! I was thinking about making it generic but couldn't came up with any nice interface.
Each import is so specific and combinations of all named/default imports + named/default re-exports would make that util function have inconvenient API.

I think some sort of searchable blog post would be helpful (this PR also should be indexed and searchable by Google and co).

@SergeAstapov SergeAstapov merged commit 46511c3 into miragejs:master Jan 10, 2022
@SergeAstapov SergeAstapov deleted the deprecate-reexports branch January 10, 2022 19:40
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