-
-
Notifications
You must be signed in to change notification settings - Fork 261
Resolve deprecations from ember barrel file #1537
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
Conversation
|
resolves #1536 |
|
|
||
| // @ts-ignore: this is private API. | ||
| const fallbackRegistry = Application.buildRegistry(namespace); | ||
| // TODO: only do this on Ember < 3.13 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this comment relevant? If so, do we even need to be supporting Ember < 3.13 here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't, but I'd argue cleanup for legacy stuff should be in a different PR (which I can submit next! as there are a number of other cleanup opportunities, I think)
| import EmberObject from '@ember/object'; | ||
|
|
||
| import Ember from 'ember'; | ||
| import { Registry } from '@ember/-internals/container'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know we specified these with the barrel deprecation RFC, but I wonder if there is anything in ember-source testing to make sure we don't break such import paths?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they're all private API, so I think it's totally fair for ember to break them if it wishes.
@ember/test-helpers should not be using private APIs haha
(we can explore migrate off of them in a separate PR)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They may be private but if we don't do that follow up PR all the pain will be ours.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aye, I'm doing this now 🎉
| // | ||
| // import { registerWaiter } from '@ember/test'; | ||
| Ember.Test.registerWaiter(this._legacyWaiter); | ||
| registerWaiter(this._legacyWaiter); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL we support the legacy waiters still
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea, and probably will continue to do so until they're deprecated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the entire legacy test system is still in there...
Co-authored-by: Katie Gengler <[email protected]>
Co-authored-by: Katie Gengler <[email protected]>
Co-authored-by: Katie Gengler <[email protected]>
No description provided.