emberjs / rfcs

RFCs for changes to Ember
https://rfcs.emberjs.com/
793 stars 408 forks source link

Moving `@ember/components` to its own NPM package? #664

Closed chancancode closed 2 years ago

chancancode commented 3 years ago

Want to gauge interest of this: are there any oppositions to requiring apps that uses @ember/component (which is probably close to 100% of apps right now) to add that package to package.json, just like @glimmer/component etc? ember-cli can even keep adding it by default for the foreseeable future.

For the foreseeable future, the actual code has to live in ember-source anyway, so all the "addon" would do in the short term is just turning on a flag in ember-source or something. But the benefit of doing this now, is that we should in theory be able to move the code from ember-source itself to the package eventually, during the 4.0 timeframe, without it being a breaking change.

Does that sound feasible? Are there any pushback on this? Is merely adding the package to package.json enough to accomplish what I wanted to do in the future?

NullVoxPopuli commented 3 years ago

Sounds great! :D

jelhan commented 3 years ago

I assume that addons using @ember/component would need to add the new packages as an explicit dependency. How long would be the transition period until all addons must be updated? Would that prevent tree-shaking of @ember/component or would there be some magic to determine if any addon need @ember/component or not? To be honest I can't remember how that was done for @ember/jquery.

dcyriller commented 3 years ago

@chancancode one question - how would it relate to the explode RFC?

ef4 commented 3 years ago

Yeah, I think @dcyriller's question is the right one. This seems fine but it needs to address the same issues as that RFC.

The two big question that come to mind are:

chancancode commented 3 years ago

@jelhan Yes addons would add it to their package.json. In terms of timeline, I think we should begin issuing deprecation warning for not having the dependency listed whenever the RFC is proposed, and it will last until whenever the next major version ships. As far as how long the @ember/component package would be supported, probably "for the foreseeable future".

@ef4 @dcyriller I think of this as an insurance policy, so we have a shot of removing the Ember.Component related code (since ember-source will soon not need them once the built-in components are modernized, which I am working on), before finishing the more difficult task of making the whole of Ember more modular. It would also be fine if we don't end up using it if we end up finishing the other task faster than expected.

As I mentioned above, yes, addons would list it as a dependency just like how @glimmer/component works today. For the foreseeable future, this addon just enables a build-time flag as the code still has to live in ember-source, so duplication is not much of an issue.

Eventually though, we may get to the point where we are able to move the component manager and super class into the addon. When we introduced @glimmer/component, we determined that the manager/class code is small enough that any duplication is probably acceptable-enough given the benefits and the state of tooling. This is probably the case for the @ember/components as well as long as we are only talking about the component manager and the super class, and the common dependencies (e.g. @ember/object, probably the event dispatcher) still lives inside ember-source.

Beyond that, it's probably deep into the territories of the explode RFC. It is also quite possible that we won't even ever get to the point where we won't bother moving any code into the addon, because the parallel effort of the explode RFC will be finished before we felt the need to work on that. Note that we will still get the possible benefit of stripping the classic component code when no one uses it even if we never actually physically move the code, which is probably the main motivation of doing this.

Either way, at that point we can integrate with whatever general-purpose solution provided by the explode RFC. If needed, we can bump the major version of the addon and start everything at 2.0 (or maybe we will want to start everything at 4.x or whatever to match the Ember version, etc).

As far as which version of the ember-source can be used with which version of @ember/component, it is a much easier problem than the general-purpose solution that the explode RFC is required to solve, at least it feels that way to me at the moment.

For the version of the addon that doesn't contain any code and just enables a build-time flag, the answer is it will be compatible with whichever ember-source (and/or ember-cli) version that allows the flag to be enabled, i.e. the ones that are new enough that has support for the flag at all, and one where the flag can be successfully enabled. Since enabling the flag amounts to calling some function at build time, wherever the flag is implemented (probably ember-source plus some infrastructure to support it in ember-cli, or maybe it'll try to reuse the optional features infrastructure internally) can throw an error and issue a deprecation warning if/when that is no longer supported, along with information on what to do. We can preemptively make sure we pass enough metadata (the package+version that requested the flag to be enabled, plus the version of the addon being used, etc) to be able to give good error messages.

For the version of that actually vendors the component manager and super class, in the scenario where we get to that before the explode RFC lands, it'll just rely on Ember having support for the component manager capabilities it needs, which is more or less the same problem that @glimmer/component (or any addon for that matter) has and has "solved". In the version of the addon that introduced the vendoring, it can detect the ember-source version, if it is new enough, it would supply the vendored code, otherwise, it will fallback to enabling the flag (in practice it'll probably have to enable some flag even when using the vendored code, to enable shared dependencies like the event dispatcher).

Some of these details are definitely more or a sketch at the moment and requires fleshing out in more details, something that I could use help with if we determined this seems plausible to do and there is enough interest to support this. I agree that if we are confident that the explode RFC + implementation would land soon, then we might not want to bother. However, given that this is a sufficiently more restricted version of the problem, and it could allow us to remove a decent amount of code for apps that doesn't want them as soon as v4.0, it feels like a pretty cheap insurance policy to have.

mehulkar commented 3 years ago

Relating #587 for bookkeeping

wagenet commented 2 years ago

Closing this for now since there's no action on it. If there is something not covered by the other RFCs, let's see about getting an additional RFC for this.