-
-
Notifications
You must be signed in to change notification settings - Fork 407
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
Native Class Roadmap #338
Native Class Roadmap #338
Conversation
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 love this! Great insight into the future of Ember.
that it fulfills a certain interface. | ||
|
||
For instance, a Component can be any class which receives an `args` hash and | ||
optionally implements any of the component lifecycle hooks: |
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.
Non important, curious question - Would this in effect reduce lookup time for properties/non existent properties, especially for things are are high up the prototype chain?
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.
In theory yes, though I wouldn't count on it being that huge of a win. In aggregate though, who knows 😄
|
||
If we go this direction, it means we cannot specify any base class behavior. One | ||
issue that arises is the fact that dependency injections will not be available | ||
to the class _during_ object construction. They can be assigned _after_ only. |
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.
Does this result affect initializers/instance-initializers?
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.
Initializers and instance-initializers are (as I understand them) essentially pure functions that run at the beginning off app creation/instantiation. They would still receive the app definition/instance and container as such, and wouldn't be directly impacted by this change.
However, and injections which are setup by initializers would be affected. All injections are currently assigned by this mechanism, and assigning after would make them unavailable.
It's worth noting that only potential solutions mentioned in the RFC which would work for injections done this way would be the init
hook and passing the injections to the constructor as named parameters. Personally, I am not a fan of the ability to unilaterally inject items onto any type thing (e.g. store
automatically injected to routes/controllers) and think it would make sense to recommend users manually declare the dependency.
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.
Perhaps in v4 we have init() to support this and in Ember v5 we drop it and make everything explicit?
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 that would be an option if we decided to go with the other DI options and drop init
altogether. It would give us a good timeline to clean things up on as well, and falls neatly inline with the proposed timeline for removing EmberObject.
Super minor text nit and personal pet peeve: “a myriad of” |
A thought on Mixin migration. I'm not 💯sure of the validity or plausibility of this, but would it make sense to have a legacy Here's an example I'm thinking (inspired by redux import Route from '@ember/routing/route';
import ApplicationRouteMixin from 'ember-simple-auth/mixins/application-route-mixin';
import applyMixin from '@ember/apply-mixin';
class ApplicationRoute extends Route {
// Stuff
};
export default applyMixin(ApplicationRoute, ApplicationRouteMixin); Or a different API might be more like (more like redux import Route from '@ember/routing/route';
import ApplicationRouteMixin from 'ember-simple-auth/mixins/application-route-mixin';
import createClassFromMixin from '@ember/create-class-from-mixin';
const createApplicationRoute = createClassFromMixin(ApplicationRouteMixin, ...any other mixins);
class ApplicationRoute extends Route {
// Stuff
};
export default createApplicationRoute(ApplicationRoute); |
Pre-removal of Post-removal of That said, I'm considering lots of tiny edge cases and small behaviors that are tied to Ember/Mixins. I think there's definitely room to make a library that applies mixins in mostly complete fashion, so legacy users can use mostly the same mixin definitions with relatively minor changes. I also think that there is room for a much better general mixin solution that actually leverages native classes (for inspiration, see this article that proposes one such system). |
The wider JS community (or at least a vocal subset) seems to like that 'functional mixin' pattern — the pipeline operator proposal calls out how it would make applying them nicer, and the original author of that post has a stage 1 proposal for formalizing them with dedicated syntax. |
@nruth And I hear rumors that prepositions are now accepted to end sentences with. (“...mere anarchy is loosed upon the world...”) I am finally old enough to experience the eternal rules of language changing in even one brief lifetime. (But of course they have ever done so...) NVM, carry on, carry on! 😀 |
``` | ||
|
||
|
||
3. **Utilize private global state to provide the container to injections during |
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 like this, but then it should be done for getOwner()
so that even a manual call to getOwner(this)
inside the constructor will work.
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 agree, this would make the most sense if we pursued this strategy. Will update the RFC to reflect that
I had a great conversation with @mike-north the other day about this, he pointed out a few concerns with this pattern going forward. I'm posting them here for discussion, but they should definitely be addressed by updates to the RFC at some point:
|
Removing the Octane label from this, as this is a more future-looking RFC that needs to be updated to reflect the current state of the world, and is unlikely to happen in the Octane timeframe. |
This RFC is pretty out if date at this point. Closing it for now, and will reopen sometime post Octane when we’ve had time to update it. |
rendered