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

Make @ember/component an optional feature #587

Closed
mehulkar opened this issue Jan 29, 2020 · 16 comments
Closed

Make @ember/component an optional feature #587

mehulkar opened this issue Jan 29, 2020 · 16 comments

Comments

@mehulkar
Copy link
Contributor

Apps that don't use @ember/component should be able to turn off the feature entirely, both as a way to signal to teammates that it should not be used for new components (old habits are hard to break) and to potentially shake that code out at build time.

@mehulkar mehulkar mentioned this issue Feb 8, 2020
@mehulkar
Copy link
Contributor Author

mehulkar commented Feb 8, 2020

In order to write this RFC, some research needs to go into:

  1. Do optional features already shake out code? If not, is that an RFC that needs to happen first?
  2. How deep does the rabbit hole go? Ember.Component extends from Ember.CoreView, which extends from CoreObject, I believe. It's possible CoreView can be dropped also? Or maybe we should refactor everything into one class first so it's easier to drop.

@lifeart
Copy link

lifeart commented Feb 8, 2020

Link-to/input/textarea should be rewritten in glimmer

@pzuraq
Copy link
Contributor

pzuraq commented Feb 8, 2020

I don't think that they should necessarily. For starters, they can't be rewritten in place transparently, because they can be imported and extended, so the classes themselves are public API.

The APIs themselves are also questionable. Input and Textarea rely on two way databinding, and LinkTo has a ton of functionality that could probably be cut down to a much more minimal core for most use cases. Another alternative would be to ship router-helpers and then not provide LinkTo in the core at all, and instead allow people to build on top of the primitives (though I think that's probably going a bit far).

I think this is ultimately a separate question from shipping the optional feature itself though. Like I said, we can't replace these classes transparently, they'll have to be added at new import paths anyways. So we can add them in a separate step from disabling the feature itself, either in parallel or later on.

@mehulkar to answer your question: The only user of CoreView anymore are classic components, so yes, they can be included I believe 😄 also, the curly component manager, and a ton of other related code. This is not at all just about removing the EmberComponent class, it's about removing all code that would be unused IMO.

@gossi
Copy link

gossi commented Feb 8, 2020

Link-to/input/textarea should be rewritten in glimmer

They can be extracted into their own addon(s). It can even come installed by default in the blueprints (for now, until there is a replacement for <LinkTo>) and then retired over time. At least, if they are a blocker to making @ember/component optional - I'd vote for that.

Anyway I'm in favor of making @ember/component optional and migrate to @glimmer/component 👍

@mehulkar
Copy link
Contributor Author

mehulkar commented Feb 9, 2020

Another bit of research that I'd like to do as part of writing this RFC is to see how much would be shaved off the vendor.js file. That's probably the biggest motivation here.

@mehulkar
Copy link
Contributor Author

^ I did some research with an Ember 3.15.0 app and was able to drop 29kb (6kb gzipped) off the vendor.js file in a new app with Ember.Component and friends removed (production build).

@lifeart
Copy link

lifeart commented Feb 28, 2020

@mehulkar as I know latest ember-source versions bundled during project build (instead of using npm bundle). I'm wondering, Is it possible to create Babel plugin to remove such code?

@mehulkar
Copy link
Contributor Author

I hadn't considered a Babel plugin, but I had explored removing it by somehow overriding the buildEmberBundles() method. or tapping into that with treeForVendor() in the app. The problem is that this is basically manual treeshaking, which effectively ends up being a soft fork. @pzuraq mentioned that core team has not wanted to support this approach in the past with any official APIs. I'm undecided about how I feel about that 😄, but it does make a lot of sense.

@mehulkar
Copy link
Contributor Author

mehulkar commented Apr 1, 2020

@locks I know the title here says optional features, but does it make sense to put this on the Deprecation Candidates board also?

@Gaurav0
Copy link
Contributor

Gaurav0 commented Apr 1, 2020

As an alternative to an "optional feature", extracting the code related to @ember/component into an addon would be almost the same thing.

@lifeart
Copy link

lifeart commented Jul 7, 2020

@mehulkar I think to start move it forward, we need to get builtin ember components ported to glimmer components.

@mehulkar
Copy link
Contributor Author

mehulkar commented Jul 7, 2020

@lifeart see pzuraq's comment above:

For starters, they can't be rewritten in place transparently, because they can be imported and extended, so the classes themselves are public API.

@jelhan
Copy link
Contributor

jelhan commented Aug 10, 2020

I don't think that they should necessarily. For starters, they can't be rewritten in place transparently, because they can be imported and extended, so the classes themselves are public API.

I'm confused. Is extending (or reopening) them considered part of Ember's public API? I thought only using them in a template is public API.

@wagenet
Copy link
Member

wagenet commented Jul 23, 2022

I'm closing this due to inactivity. This doesn't mean that the idea presented here is invalid, but that, unfortunately, nobody has taken the effort to spearhead it and bring it to completion. Please feel free to advocate for it if you believe that this is still worth pursuing. Thanks!

@wagenet wagenet closed this as completed Jul 23, 2022
@NullVoxPopuli
Copy link
Contributor

I think this is also planned work, we're just not there yet

@wagenet
Copy link
Member

wagenet commented Jul 23, 2022

My impression here is that most, if not all, of the necessary work to make this happen is encapsulated in other efforts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants