rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

Splitting Ember into packages

Open ef4 opened this issue 7 years ago • 53 comments

Rendered

ef4 avatar Dec 13 '17 18:12 ef4

Beautiful!

knownasilya avatar Dec 13 '17 19:12 knownasilya

It's unclear to me from the RFC, but will Semantic Versioning be respected in @ember/core?

Given the given example from the RFC, let's assume Ember 3.2.0 is released and it splits out @ember/partial and @ember/prototype-extensions into their own packages. That means @ember/core no longer has the same API surface area as it did before, so it is a breaking change. Does that mean that @ember/core's major version number will have to be bumped? If so, that means the @ember/core version number will quickly fall out-of-sync with the ember-source version number. If not, that means @ember/core would not follow semver.

What are the trade-offs between those, and which is the current plan?

Kerrick avatar Dec 13 '17 20:12 Kerrick

I find some are unclear for addon authors. Today is common and a good practice to test your addons in beta and canary versions of Ember. The Scenario: Addon author declaring dependencies doesn't clarify enough how the required-base-features will work when testing with a version beyond stable. I have the feeling that if we don't handle that gracefully, people who want to ride the beta-wave (not to mention canary) will have to either use ember-source to be safe, or every time they update to a new beta, as the ecosystem will naturally lag behind, they have to heavily edit the config/dependency-overrides.js to satisfy all the unmet new packages.

This RFC is pretty dense, so I might just as well be misunderstanding something.

cibernox avatar Dec 13 '17 20:12 cibernox

Am I in the minority of wanting one "monolithic" ember package? I'd rather have everything "ember" at my disposal, then have tree-shaking remove the un-needed stuff on build (shake not only my app code, but what pieces of ember are not being used by the app and addons, etc.) Just seems like this would add so much more complexity when tree-shaking could take care of the job. Or am I missing something?

Panman82 avatar Dec 13 '17 20:12 Panman82

@Panman8201 that is achieved here. If you want "everything ember" you'll continue to use ember-source if you want to explicitly declare your dependencies (or dare I say add Ember features to a Glimmer app in the future) then you can include only the bits you want.

rtablada avatar Dec 13 '17 21:12 rtablada

@Panman8201 I think you are definitely in the majority. I think most users wouldn't have to worry about this at all, and as long as they continue to use ember-soure (maybe rename to something like @ember/all in the future) things will work exactly the way they are today. That should still be the default in guides, CLI blueprint etc.

You only need to do this if you are very filesize-conscious (I personally think most apps – at least most "productivity" kind of desktop apps aren't in that category). But it's nice that people in that category can do it if they want to.

chancancode avatar Dec 13 '17 21:12 chancancode

Also, I don't think tree shaking can be very effective at removing the kind of features we are talking about here. For example, Mixins, computed properties and observers are all features that are deeply weaved into different parts of the framework. Even if you are not importing the macros in your app the Ember.Object (for example) will have to handle all these possibilities in the constructor, so these things still ended up being imported, so from the bundler's perspective they are all "used". Template features like partials are also very difficult to tree-shake, since you don't even have an import statement.

Not that it's impossible, but the bundler would have to get really smart and have a lot of knowledge about Ember's internals to do it correctly.

chancancode avatar Dec 13 '17 22:12 chancancode

@Kerrick

Critically, @ember/core includes an extra rule in its semver policy: the only supported way to upgrade to a newer @ember/core is to run the ember-cli-update command. The update command will be aware of any additional packages have been split off from @ember/core since your last version, and it will add them automatically to your app.

When you run the upgrade command, it will edit your package.json such that the newly split packages are included in your package.json by default, so it behaves exactly the same way after the upgrade.

chancancode avatar Dec 13 '17 22:12 chancancode

When you run the upgrade command, it will edit your package.json such that the newly split packages are included in your package.json by default, so it behaves exactly the same way after the upgrade.

@chancancode I understand that the proposal includes an upgrade command to help manage breaking changes in @ember/core that result from feature extraction, but will that tool be upgrading the @ember/core package's major or minor version?

A potential problem with this being a minor version bump to @ember/core is that traditional semver does not consider the use of external tools to maintain compatibility -- it only considers version numbers. So this could introduce compatibility issues with other external tools that rely on semver.

Kerrick avatar Dec 13 '17 22:12 Kerrick

@cibernox that is the point of the "required-core-features" system. It is basically saying "this addon was last audited as of version 3.10" – so if version 3.11 splits out two additional packages (say @ember/computed-properties and @ember/observers), then we will conservatively assume the addon requires those features by default.

So everything will still work, but your build might not be as lean as your hoped (it will be no worse than before the upgrade, though). You can use the override feature to test if anything breaks if you insist to remove those features and send a PR to the addon to "re-certify" it against 3.11.

chancancode avatar Dec 13 '17 22:12 chancancode

@Kerrick at the moment ember-cli-update has from and to flags, for specifying versions, so I'm assuming those will stay or additional flags would be added as shortcuts, e.g. --major, --minor, or --patch.

knownasilya avatar Dec 13 '17 22:12 knownasilya

The problem with this is that semver does not consider the use of external tools to maintain compatibility -- it only considers version numbers.

@Kerrick I don't think that is true.

The only relevant concept in Semver is "public API", which is intentionally very loosely defined:

Software using Semantic Versioning MUST declare a public API. This API could be declared in the code itself or exist strictly in documentation.

As part of the documentation, this piece of software communicates to the users that "the only supported way to upgrade is to use the upgrade command", which does whatever it needs to do to maintain the API compatibility for you.

It is also absolutely not true that you don't have to use "external tools" to maintain compatibility. In practice all node packages have the implicit requirement of "the only supported way to upgrade is to use an NPM-compatible package manager" (where "NPM-compatible package manger" is not even that well defined).

If you try to upgrade a package simply by downloading the tarball of a newer minor version and extract that into the appropriate location in your node_modules, it will likely not work because you will be missing some newly required dependency.

It is just a matter of clearly defining what you consider the "public API" and communicating that clearly with your users, which is why even things like legal documents could be considered "semver-compatible".

chancancode avatar Dec 13 '17 22:12 chancancode

and FWIW I tend to agree with @Kerrick that @ember/core would very likely not follow semver if implemented in lockstep with ember-source

Turbo87 avatar Dec 13 '17 22:12 Turbo87

What @chancancode said in https://github.com/emberjs/rfcs/pull/284#issuecomment-351539299 was quite helpful to me to clarify the difference to only relying on tree-shaking. Maybe something like this could be added as another "How this relates to" section?

simonihmig avatar Dec 13 '17 23:12 simonihmig

This seems cool and I could see how it could make the life easier for the maintainers of Ember. That said, it seems like it would be a lot of effort and I'm struggling to see the real value it offers to Ember's end users.

workmanw avatar Dec 14 '17 01:12 workmanw

Given the given example from the RFC, let's assume Ember 3.2.0 is released and it splits out @ember/partial and @ember/prototype-extensions into their own packages. That means @ember/core no longer has the same API surface area as it did before, so it is a breaking change. Does that mean that @ember/core's major version number will have to be bumped? If so, that means the @ember/core version number will quickly fall out-of-sync with the ember-source version number. If not, that means @ember/core would not follow semver.

This is a misunderstanding of semver. "Public API" it not limited to things like which functions are available. It can include whatever other conditions you want to impose. What is important is that you state clearly what is the supported way to use the software, and anyone who follows those instructions is safe.

So no, we would not bump major versions of @ember/core when splitting out packages. That is basically the whole point of this RFC. We can just start splitting packages out of ember-source right now if we're willing to only do it at breaking releases.

ef4 avatar Dec 14 '17 02:12 ef4

Today is common and a good practice to test your addons in beta and canary versions of Ember. The Scenario: Addon author declaring dependencies doesn't clarify enough how the required-base-features will work when testing with a version beyond stable. I have the feeling that if we don't handle that gracefully, people who want to ride the beta-wave (not to mention canary) will have to either use ember-source to be safe, or every time they update to a new beta, as the ecosystem will naturally lag behind, they have to heavily edit the config/dependency-overrides.js to satisfy all the unmet new packages.

The important thing here is that in none of those situations is anybody worse off than they are right now.

Even if you're using @ember/core and removing some packages, you can always safely upgrade to a newer version with more split-out packages regardless of what addons you're using. The upgrade will simply put those packages into your app's dependencies and everything continues working the way it did before.

The only time you need to do extra work is if you get motivated to try to remove some of those newly split-out packages from your app.

ef4 avatar Dec 14 '17 02:12 ef4

This is a misunderstanding of semver.

If it's a misunderstanding of semver, it is likely a common one. Everything from third-party tools like David DM and GreenKeeper to the ^ used by yarn and npm assumes that it's safe to upgrade from 3.2.0 to 3.6.0 without breaking your build. That assumption would no longer apply if upgrading @ember/core from 3.2.0 to 3.6.0 would break without either using the ember-cli upgrade tool or manually adding more dependencies.

you can always safely upgrade to a newer version with more split-out packages regardless of what addons you're using.

*If you use the built-in tool and don't use any third-party tooling that has been built around the common community understanding of semver.


I'm all in for splitting into multiple packages, but I think that serious thought needs to be given to allowing the major version numbers of ember-source and @ember/core to drift. Is it really so bad that [email protected] depends on @ember/[email protected], @ember/[email protected], etc?

Google does a similar thing with material-components-web -- each component (equiv. to @ember/core or @ember/string) releases a major version number when its own API has breaking changes, and the "wrapper library" (equiv. to ember-source or the proposed @ember/all) has its own separate release cycle.

Kerrick avatar Dec 14 '17 02:12 Kerrick

This seems cool and I could see how it could make the life easier for the maintainers of Ember. That said, it seems like it would be a lot of effort and I'm struggling to see the real value it offers to Ember's end users.

I agree with @chancancode that many of the apps that are using Ember today shouldn't care about this at all. You're not wrong when you question the value to current Ember users.

But the goal here is to enable a whole new class of apps to become Ember users. Apps that today would not have chosen Ember because it's "too big" or "too much to learn". We can create a more comfortable adoption path for all those apps.

ef4 avatar Dec 14 '17 02:12 ef4

But the goal here is to enable a whole new class of apps to become Ember users. Apps that today would not have chosen Ember because it's "too big" or "too much to learn". We can create a more comfortable adoption path for all those apps.

If the hope is that these sub-packages can become an adoption path for new users to the Ember ecosystem, it’s even more important that they follow the common version of semver on their own, without breaking changes inside a single major release.

Let’s say a creator of that whole new class of apps decides to use @ember/[email protected] and installs it as ^3.6.0 in their non-Ember app. They don’t use ember-cli-upgrade because they don’t use Ember CLI or most of the rest of the Ember ecosystem.

Now let’s say that @ember/[email protected] removes the pluralize and singularize methods to extract them into @ember/inflection instead. Now we’ve got a user who has a broken application because their build system auto-updated @ember/string from 3.6.0 to 3.7.0 which removed public API methods. That user feels burned by this, so they may never continue exploring Ember and become a user.

In an alternate world, @ember/string was bumped to 4.0.0 instead of 3.7.0 when removing those methods. The user’s app never broke, and their tooling let them know that @ember/string was out-of-date. They read the release notes and see a note about installing @ember/inflection if they want to keep the removed methods. The user can make an informed decision and feels much more confident in the Ember ecosystem than if their build had broken out from under them.

Kerrick avatar Dec 14 '17 02:12 Kerrick

@Kerrick ah, you are hitting on some important points I did not consider.

I should probably say explicitly that the upgrade rule applies to @ember/core itself only. We would not be splitting code out of @ember/string without bumping major versions.

Your point still stands that @ember/core having a weird upgrade policy is suboptimal from a compatibility-with-the-wider-world perspective. We just face a fundamental tradeoff here. We have a particular class of changes we want to make that can be 100% automated in a reliable way. And we will always have another class of changes that are truly breaking. We need to communicate the difference to users.

We are also working within the limitations of NPM, which makes meta-packages not really work well. It would be ideal to just have an ember package that depends on all the others, such that we can rearrange the others however we want, and users who depend on the meta package wouldn't care. The problem is that you can't actually force npm to do that properly unless you do it with peerDeps, and npm gives you no help in solving the constraint problem of getting a set of peerDeps that all approve of each other.

If somebody wants to figure out how to make a meta package work well, I would be happy with that alternative. I have gone pretty far down the path of trying to design that, and each time discovered I would need to do "weird" things that NPM doesn't normally do to make it work.

ef4 avatar Dec 14 '17 02:12 ef4

What's the minimum package set that makes Ember, "ember"? I had thought until now that ember-source was the most minimal. What limited subset would @ember/core actually contain? (And would any of the other exploded packages actually be "ember"? E.g. @ember/string doesn't sound like ember, it sounds like a string library that happens to be published in ember's namespace, and that maybe Ember.js the framework uses.

mehulkar avatar Dec 14 '17 07:12 mehulkar

What's the minimum package set that makes Ember, "ember"? I had thought until now that ember-source was the most minimal.

That is the question, isn't it ;)

How I view this initiative is that we want to allow users to opt into a smaller subset of Ember than ember-source. I think the examples in the RFC of @ember/partials and @ember/prototype-extensions illustrate nicely the sort of packages that a user might want to opt out ahead of their removal from ember.js, which would also be hard to do via tree shaking mechanisms as their use is fairly implicit.

(And would any of the other exploded packages actually be "ember"?

I am not sure I fully understand this point, as we have already broken Ember into separate packages. What does "being ember" mean to you?

E.g. @ember/string doesn't sound like ember, it sounds like a string library that happens to be published in ember's namespace, and that maybe Ember.js the framework uses.

Hhhmm… ;)

locks avatar Dec 14 '17 08:12 locks

If you can folks make it possible to extract ember-data outside ember to use it as a standalone API wrapper, I think it will be huge win. Apart from that, splitting ember into packages seems quite reasonable and the way to go.

vasilakisfil avatar Dec 14 '17 09:12 vasilakisfil

I do understand and agree that this RFC is in line with "vanilla" SemVer, but as @chancancode wrote:

In practice all node packages have the implicit requirement of "the only supported way to upgrade is to use an NPM-compatible package manager" (where "NPM-compatible package manger" is not even that well defined).

So, in essence, this proposal would introduce a way to use Ember, that is in fact outside the "NPM-compatible package manger"-ecosystem, but looks weirdly like it was part of it. @Kerrick has given a perfect example (Strings etc) how that would hurt. One way to deal with this would be to have @ember/core versions out of sync with ember-source etc. As I understand that this hurts in other ways, why not start with 0.x.y versions for @ember/core while there are still many packages factored out, and start stable version numbers in sync with ember-source when the main work is done, but in accordance to the classic npm notion of SemVer?

jnfingerle avatar Dec 14 '17 11:12 jnfingerle

@ef4

But the goal here is to enable a whole new class of apps to become Ember users. Apps that today would not have chosen Ember because it's "too big" or "too much to learn". We can create a more comfortable adoption path for all those apps.

Honestly this feels like a facade. I can't imagine someone deciding, "I'm just going to pick up the Ember core and the router, but not use services or components". Because even if these things are decoupled at the package level, they're still part of the same ecosystem. Guides, Docs, Tutorials, Addons, etc would all have to take extreme care to explain and enforce this.

You're not wrong when you question the value to current Ember users.

It's also about the time and resources that go into this project that could be spent elsewhere. Not just the author's time, but those who review code changes, testing, documenting, reporting bugs that arise, etc.

workmanw avatar Dec 14 '17 12:12 workmanw

@workmanw

Honestly this feels like a facade. I can't imagine someone deciding, "I'm just going to pick up the Ember core and the router, but not use services or components"

The likely scenario is to start with just components. @ember/core would converge with GlimmerJS.

I absolutely agree with the premise that most real apps will eventually need most of what's in Ember. But the problem is lots of people don't understand that when they're getting started, so they pick a component-only library over Ember, and then end up reinventing all the other stuff as they go, much to their lost productivity and stability, and to our loss as a smaller community.

It's also about the time and resources that go into this project that could be spent elsewhere

This proposal is deliberately intended as infrastructure that should make everything else go faster, not slower. That is why I wrote this RFC in the first place. I don't happen to be a user that is obsessed with byte size. I'm motivated by strategic efforts to help the whole community ship faster.

There is still plenty of old and broken stuff floating around in Ember that nobody has stepped up to fix because there's no near-term payoff, since even deprecated stuff needs to stay around for quite a long time. This proposal creates a near-term payoff to fixing.

It also gives us general-purpose tools for backward compatibility, such that all future changes get easier to make, because "how to swap out the old code without breaking apps" becomes a problem with a standard solution. This lowers the amount of effort required for lots of features that people want.

It also creates a clear path for advanced users to experiment with next-generation replacements for things like the router. The more experimentation we unlock, the faster we can stabilize new features and bless them as the official release.

Your questions are good ones and they are forcing me to be more clear than I was in the original doc. The highest-level goal of this proposal is to make it easier to ship improvements in Ember, so that more people can overcome the intimidation and effort barriers required to help.

ef4 avatar Dec 14 '17 14:12 ef4

Am I in the minority of wanting one "monolithic" ember package? I'd rather have everything "ember" at my disposal, then have tree-shaking remove the un-needed stuff on build (shake not only my app code, but what pieces of ember are not being used by the app and addons, etc.) Just seems like this would add so much more complexity when tree-shaking could take care of the job. Or am I missing something?

I want to echo @chancancode. I think for apps that don't already have a reason to trim down Ember, the "single package" approach is working very well, and it's something we want to continue to support. For what it's worth, Skylight (Tilde's main app) is such an app.

Ember is now and will always be a "convention over configuration" framework first, and we (or at least I) have no desire to force a menu of choices on our users if they don't want it.

On the other hand, people are increasingly trying to use Ember in more resource-constrained mobile environment, and in practice those users are doing very aggressive (and very non-standard) things to cut down the size of Ember.

This RFC is targeted at those users, and tries very hard to avoid leaking those concerns to the majority of users who are not very concerned about this issue.

It also provides an opt-in (and compatible) way for addons to declare what features they need, so that addons can help out apps that are trying to minimize their payloads.

My expectation is that addon authors would be receptive to being more explicit about which features they use in order to help out apps that really need it, but addons can continue to assume "all of Ember" compatibly if they want.

wycats avatar Dec 14 '17 15:12 wycats

@jnfingerle I appreciate your focus on semantic versioning here, as I think it's a very important constraint.

I think it would be very productive to get specific about the ways in which you think this proposal diverges. For what it's worth, @ef4, @chancancode and I thought a lot about semantic versioning, but reading your comments, it's clear we made some mistakes.

It might just be that we got too clever or subtle with the way we interpreted semantic versioning, it may be that we explained it poorly, or it may be that we simply were wrong about the details.

I'm definitely interested in drilling in harder: can you be more specific about where you feel the semantic versioning holes are?

wycats avatar Dec 14 '17 16:12 wycats

Honestly this feels like a facade. I can't imagine someone deciding, "I'm just going to pick up the Ember core and the router, but not use services or components"

Over a longer time horizon, I suspect that we will be able to get away with using tree shaking and more traditional packaging strategies to deal with this set of problems.

However, Ember itself was not written with those strategies in mind (in fact, it was written at a time where the opposite constraints were common). The point of this RFC is largely to find a transition path from Ember's original implementation strategy towards the strategies that people intuitively prefer.

A lot of the friction here comes from the fact that in order to make Ember more "pick and choose" we need to disentangle parts of Ember that have more-or-less been entangled for its entire history. Because so much of the ecosystem implicitly depends on the entanglement, it's not always possible to transparently allow people to opt out of parts of Ember without impacting other users compatibly.

The goal of this RFC is to create a more explicit way for apps that care about "opting into a smaller build" to do so very explicitly, but without changing the standard way of using Ember.

Let's keep driving towards a way to break up Ember into smaller pieces for people who want to use a smaller subset of Ember, while at the same time avoiding impacting mainline Ember use. If you feel weirded out by this RFC, it's because there are real risks to this change, but I think it's one we need to do, and I also think that we can make it nearly invisible to today's Ember users with some design effort.

wycats avatar Dec 14 '17 17:12 wycats

Honestly this feels like a facade. I can't imagine someone deciding, "I'm just going to pick up the Ember core and the router, but not use services or components"

To say a bit more about my previous comment (I realize it was implicit), while it might be hard to imagine using Ember without "the router", it's not too hard to imagine using Ember without "renderTemplate", but a lot of those features are deeply entangled in the current code.

The primary goal of this RFC is to allow people to use "modern Ember style" even if it's very hard to technically separate the code. We want new features to lean on using ES6 module imports and normal tree shaking techniques, but we need a strategy that were written without that constraint in mind.

As an example future item, we might want to allow people to use Ember without Ember.Object (optionally), and migrate to an ES6 class style. We wouldn't want to deprecate Ember.Object quickly, because of how disruptive that would be to today's applications, but we need a mechanism for letting people opt out of including the Ember.Object code (a nontrivial amount) if they are willing to write code without Ember.Object.

wycats avatar Dec 14 '17 17:12 wycats

We want new features to lean on using ES6 module imports and normal tree shaking techniques, but we need a strategy that were written without that constraint in mind.

Isn't that going backwards? Sub-packaging would have been good for the "globals" days, but imports + tree shaking is the way forward.?.

Wouldn't this also put more strain on the release folks?? Instead of releasing one package, now its many packages. Don't want to over extend peoples time..

And for releases, will all package version numbers be bumped for a new release, even if nothing has changed? Ex: It would be odd to see the (latest versions) router at 3.2.4 and controller at 3.2.0.

To add, I've been concerned about ember changes before (even prior to RFC's), but they worked out Ok in the end. So I'm not overly concerned about this, just concerned that it'll add more complexity than what might be needed.

Panman82 avatar Dec 14 '17 17:12 Panman82

Isn't that going backwards? Sub-packaging would have been good for the "globals" days, but imports + tree shaking is the way forward.?.

We need to split off the (finite) code that's hard to move forward with so that we can move forward with more modern techniques going forward.

The intent is not to keep doing the package strategy indefinitely. The intent is to use the package strategy a few times to bootstrap a codebase that can use more modern strategies.

wycats avatar Dec 14 '17 19:12 wycats

I honestly think that using 0.* version numbers for @ember/core and other subpackages, until they are stable and unlikely to break up further, might be an acceptable compromise over the semver issue.

To what extent does having parts of Ember have lockstep versioning with the overall Ember package really provide value? We could consider keeping @ember/core's version be the version of ember-source minus 3.

Gaurav0 avatar Dec 14 '17 19:12 Gaurav0

Thank you everyone for your input. I just pushed an update to the document that takes into account what I see as the two biggest points raised in this discussion:

  • because my proposed semver rules for @ember/core were too nonstandard, I changed the proposal to say that we should start at 0.1.0 for all the split out packages and signal breaking changes each time we split out a new package. Only after stabilizing all the package boundaries would we jump ahead to match version numbers with ember-source again.

  • to avoid burdening addons during the initial unstable phase, I updated the local overrides section to also include a shared community package for tracking addon feature dependencies. This would allow the early adopters of @ember/core who are motivated to get their apps working with less Ember packages to do most of the work without blocking on addon authors.

ef4 avatar Dec 17 '17 04:12 ef4

@wycats

I'm definitely interested in drilling in harder: can you be more specific about where you feel the semantic versioning holes are?

For completeness' sake: I saw the problem with introducing a new SemVer rule. While the result was still a valid variant of SemVer, it diverged from conventional npm/package.json SemVer, basically breaking SemVer for anyone not aware of the additional rule. This anyone could have been people (newcomers to the Ember community not yet educated, package users from the wider node/npm community not interested to go full Ember) as well as tools targeted to the npm ecosystem that Ember users might want to use.

That said, all this has been fixed in the new version @ef4 pushed yesterday.

jnfingerle avatar Dec 18 '17 10:12 jnfingerle

The new version @ef4 pushed addresses my concerns. Love this RFC now!

Kerrick avatar Dec 18 '17 17:12 Kerrick

Thanks to everyone who contributed so far! The Core team discussed this RFC at the last meeting and there was consensus on moving it to FCP, so here we go :)

locks avatar Jan 24 '18 02:01 locks

I believe there were some good points brought up by @Turbo87 above that haven't been addressed regarding peer deps.

kellyselden avatar Feb 04 '18 23:02 kellyselden

@kellyselden

At least once I got peer deps unsatisfied warnings from NPM, where I knew my setup was more correct than NPM's algorithm

So here's the thing: virtually nobody can reasonably make this part of their normal workflow. Trying to manually resolve graphs of dependencies, while allowing duplicates "when appropriate", and resolving peer dependencies manually at the top level is just a nonsense requirement that nobody can do.

The original version of this proposal tried to avoid that problem by moving the resolution for this particular problem into side-configuration, so that Ember could handle the resolution for you and avoid the brokenness of npm's peer dep warnings.

However, there was strong pushback that we should just "go with npm" and not "reinvent the wheel". I'll accept either of the two, but if we're gonna go with peer dependencies, we need to be able to assume they work. If we think people will need to manually do work to get around brokenness, we really need to use the side-config approach.

So which is it?

wycats avatar Feb 05 '18 03:02 wycats

@wycats

I'll accept either of the two, but if we're gonna go with peer dependencies, we need to be able to assume they work.

If npm really is that broken it has to be fixed. If that isn't possible, by all means, run your own but don't do it inside the existing npm ecosystem disguised as part of it when it isn't.

Maybe the peerDependencies approach with an "I know what I'm doing and don't want hard errors" config option might be a solution?

jnfingerle avatar Feb 06 '18 09:02 jnfingerle

Maybe the peerDependencies approach with an "I know what I'm doing and don't want hard errors" config option might be a solution?

That's essentially what I meant with "escape valve" in https://github.com/emberjs/rfcs/pull/284#discussion_r156804814

One example where the npm peer deps warning was entirely wrong for me:

  • eslint-plugin-qunit (with "eslint": ">=3.18.0 <5.0.0" peer dep)
  • ember-cli-eslint depends on broccoli-lint-eslint depends on eslint@4

The nested eslint version was correct and satisfying the eslint-plugin-qunit peer dep constraint, but npm was still complaining because I don't have eslint as a direct dependency.

These things seem to happen quite often to me and I'm worried that enforcing peer deps in the same way will create much more pain than it solves. I'm absolutely okay with displaying warnings for them, but I'm opposed to hard errors if there is no way to turn them off.

Sidenote: we should consider how to implement this in Ember CLI without losing startup performance

Turbo87 avatar Feb 16 '18 10:02 Turbo87

@Turbo87 In your eslint example, isn't the issue resolved by including eslint as a direct dependency of the app? To me, this seems okay because there are no guarantees that a future release of broccoli-lint-eslint won't change its version to something that no longer satisfy's eslint-plugin-qunit's peer dependency.

In other words, it seems okay to me to say that transitive dependencies are not allowed to fulfill peer dependency requirements. The worst case scenario is you can just adopt the dependency with the same version range as the other dependency bringing it in transitively. In your example, you could add eslint@4 to your package.json and dependencies would resolve the same as before, but now satisfy the peer dependency.

Is there a downside to this that I'm missing?

tomdale avatar Feb 19 '18 19:02 tomdale

isn't the issue resolved by including eslint as a direct dependency of the app?

then something like yarn run eslint would potentially run a different ESLint version than the one that broccoli-lint-eslint is resolving to, which would not be such a great solution unfortunately.

this seems okay because there are no guarantees that a future release of broccoli-lint-eslint won't change its version

I think the expectation is that if broccoli-lint-eslint (and ember-cli-eslint) update to a new major eslint version they would also bump majors. They are essentially exposing the ESLint APIs directly to the users so any breaking API change (including rule changes) from eslint would also be a breaking change for those libs.

In your example, you could add eslint@4 to your package.json and dependencies would resolve the same as before, but now satisfy the peer dependency.

Until someone updates eslint to 5.0.0 and forgets about ember-cli-eslint. Essentially it's just a workaround for the "broken" warnings in npm and yarn, but at least they're just warnings, not hard errors.

Turbo87 avatar Feb 19 '18 19:02 Turbo87

Removing Final Comment Period so that we can address recent feedback...

rwjblue avatar Mar 09 '18 22:03 rwjblue

Has the feedback been addressed?

NullVoxPopuli avatar Sep 18 '18 13:09 NullVoxPopuli

Update: this RFC has been on the back burner for a while but it is still quite relevant and the text holds up well. I still think it's a good idea, I just haven't been actively championing it as working on embroider comes higher in my priorities list.

The only major remaining objection was concern about making invalid peerDependencies into errors. I accept that the NPM ecosystem is riddled with problems that make keeping peerDeps valid difficult in general. My suggestion is:

  • reduce the scope of that feature so that it only errors on unsatisfied peerDeps in Ember's own packages.
  • provide an environment variable to suppress the error so that people who are doing advanced debugging of the ember packages themselves can test scenarios that involve mismatched package versions.

If anybody has some time to take over and incorporate that into the text, I think this RFC is still right near the home stretch.

ef4 avatar Mar 20 '20 19:03 ef4

If the latest feedback has been addressed by https://github.com/emberjs/rfcs/pull/646, it is OK to move this RFC along its path to be merged?

hakilebara avatar Jul 20 '20 16:07 hakilebara

What's left on this? :thinking:

NullVoxPopuli avatar Jan 29 '22 17:01 NullVoxPopuli

What's left on this? 🤔

Has implementation even started? As far as I know ember-source is still the only way to depend on Ember.

Some preparation work has been started for sure. Some features earlier included in ember-source package have been replaced by standalone packages. @glimmer/component, @glimmer/tracking and @ember/test-helpers are some examples.

But a common Ember all still relies heavily on fake packages like @ember/service, @ember/routing and @ember/controller. All of them are still provided by ember-source.

It even gets worse if also pulling all the fake packages into account, which are less commonly used by an application itself. By far the most packages listed in Ember API docs are fake packages provided by ember-source. It's still a long, long way until ember explode is possible as described in this RFC.

But to be honest I'm not sure if that's still the right target. Maybe instead of splitting up existing Ember we could continue to implement new replacement as standalone packages. It's a slow transition path.

But I'm also not sure if there are that many use cases which require a developer to start with @glimmer/component and install all their way up to Ember. If it is about bundle size, maybe we could achieve the same quicker focusing on strategies like dead-code elimination (tree-shaking), deprecating features in core like @ember/component and excluding them from the built based on optional features.

I don't have much insights in core development. But it seemed as if doing so worked out quite well in the last two years.

jelhan avatar Jan 29 '22 21:01 jelhan

I still think the high-level goals of the RFC are good. I think we would want to do an updated iteration that takes advantage of all the progress we've already made recently across the ecosystem.

For example, we're starting to see addons shipping in v2 format now. Since V2 addons have to ship their code in a form that's statically analyzable, we can reliably tell which Ember packages they really use. So they wouldn't need the required-core-features and required-additional-features stuff that's in this RFC.

As of now, that would cover addon usage of Ember modules via JS imports, but when you combine it with https://github.com/emberjs/rfcs/blob/master/text/0496-handlebars-strict-mode.md via something like https://github.com/emberjs/rfcs/pull/779, we would have full visibility into what parts of Ember are used by any addon. That solves the hard migration problem that this RFC deals with.

Also, as of Ember 4.0 we made sure that apps are required to have support for v2 addon format via ember-auto-import >=2. That means we're free to ship ember-source itself as a v2 addon, which makes all usage of ember modules pay-as-you-go.

So I think we're actually making huge progress in moving the ecosystem toward making it possible to ship ember as "real" packages without disruption.

The next ecosystem-wide thing I'd like to do that would protect users from breakage as you move toward ember being "real packages" made out of "real modules" is deprecating all usage of require and define. But we need to ship the replacement first, which probably means making our DI system asynchronous and enabling top-level await.

ef4 avatar Jan 30 '22 22:01 ef4

If we are considering a revised RFC around this concept, would we ever consider allowing for the new "isolated" modules to not require glimmer as it's rendering engine? Or will the packages always require the assumption that the rendering engine/library is glimmer.

webark avatar Jan 31 '22 00:01 webark

@ef4 should we leave this open still?

wagenet avatar Jul 23 '22 23:07 wagenet