Ember CLI build pipeline

[mehulkar]

Rendered

[Gaurav0]

As written, I find the motivation section less than compelling. This does not mean I do not support your proposal, rather that it needs to be improved with specific use cases and answering more questions.

Currently, the only supported way to affect the Broccoli tree during build time is by hooking into the treeFor* APIs provided by addons.

This is not true. See Ember Twiddle's ember-cli-build.js to see how one can affect the final build output without using the treeFor* apis. This should be documented, but never has been. Please explain why this is not enough.

An addon is a full npm package. Even if it’s an in-repo addon, it comes with its own name and dependency tree. The burden of maintaining another package to write a few lines of code perpetuates the perception that Ember is "bloated" and has a steep learning curve.

This may be more a documentation problem and a lack of tutorials than an issue with creating addons. Having experience in this, I find the process easy thanks to the available addon blueprint. Please explain why having these hooks allow more flexibility and control. Add specific use cases where using an addon is burdensome.

There is no guarantee of when or how the code will be called, so it keeps developers locked into the Ember ecosystem. This feeling of being trapped in a build system means under-investment, or worse: moving away.

The before and after keys in package.json of ember addons allow for controlling the order in which addons run. Please explain why the before and after hooks are insufficient.

The entry point of the Ember build is the ember-cli-build.js file. The full power of a good program should always be available in the entry point of the program. Being able to add modules into the program should be treated as a progressive enhancement rather than a requirement.

ember-cli, like broccoli itself, should do nothing without addons. To the extent that it does, IMO that logic should be moved to addons, because ember-cli can be used to build anything, not just ember apps.

there is no good way to guarantee the order of these addons.

Please explain why the before and after keys in package.json are insufficient.

Any framework-agnostic business logic, configuration, or data needed by the build pipeline must first be deployed independently as a module and then wrapped again in an ember-specific addon. This is a maintenance nightmare.

Please explain why this in particular is a maintenance nightmare. We've been doing this for over 6 years.

[mehulkar]

Thanks for the detailed feedback @Gaurav0!

I'll respond to some of your feedback here before updating the RFC text, if that's ok.

This is not true. See Ember Twiddle's ember-cli-build.js to see how one can affect the final build output without using the treeFor* apis. This should be documented, but never has been. Please explain why this is not enough.

I suppose it's possible, but under-documented. I've been told in Discord that an Addon is the "right" way to add to the build, and I should not try to mess with the ember-cli-build.js file too much. This is why I said addons are the only "supported way". It would be a good step forward to improve documentation around the EmberApp class and how to use the trees more correctly, but at the same time, I don't think it's reasonable to ask a developer to learn everything about Broccoli before being able to do something as simple as write a file at build time. (which I've sadly tried and failed to do several times). treeForPublic for example seems simpler because at least I can guess that it will affect public/.

I find the process easy thanks to the available addon blueprint. [...snip...] Add specific use cases where using an addon is burdensome.
Please explain why this in particular is a maintenance nightmare. We've been doing this for over 6 years.

We have an NPM module fetches translations. This module is used in non-Ember contexts, so we wrap it in an Addon for Ember. If we need to make a change to this module, it means making 3 PRs and 2 publishes in a very specific order. Without the middleman, it could be 2 PRs and 1 publish. We have a surprising number of addons whose only job is to do this kind of wrapping.

ember-cli, like broccoli itself, should do nothing without addons.

I'm not sure I agree with this, but the main driver is that any system should have a story for the simplest possible use case. If "plugin everything" is the direction to go, addons should be simplified and taught progressively, starting from a single file, and that's what the blueprints should output also. Even so, I think the final developer experience would look like this, making it possible to load and run build time code without understanding the whole system.

const allAddons = require('ember-find-and-load-addons')();
const someFunctionality = require('third-party-lib');
return new EmberApp({
  addons(env) {
    return [
      someFunctionality(),
      ...allAddons
    ]    
  }
}

app.toTree();

[Gaurav0]

I suppose it's possible, but under-documented. I've been told in Discord that an Addon is the "right" way to add to the build, and I should not try to mess with the ember-cli-build.js file too much. This is why I said addons are the only "supported way". It would be a good step forward to improve documentation around the EmberApp class and how to use the trees more correctly, but at the same time, I don't think it's reasonable to ask a developer to learn everything about Broccoli before being able to do something as simple as write a file at build time. (which I've sadly tried and failed to do several times). treeForPublic for example seems simpler because at least I can guess that it will affect public/.

It's not underdocumented, it's not documented at all. :) Using addons is indeed the more supported way. Using the treeFor* hooks. I think we need some api here and replicating the existing treeFor* hooks may not be the best api, as these are pretty opaque and still require knowledge of broccoli. Again, it would help to show a specific use case in your motivation section, explain why treeFor* hooks would help. With a use case to discuss, we can demonstrate why this is necessary.

We have an NPM module fetches translations. This module is used in non-Ember contexts, so we wrap it in an Addon for Ember. If we need to make a change to this module, it means making 3 PRs and 2 publishes in a very specific order. Without the middleman, it could be 2 PRs and 1 publish. We have a surprising number of addons whose only job is to do this kind of wrapping.

This is an excellent explanation. Just explain why you can't use an in repo addon or monorepo here.

ember-cli, like broccoli itself, should do nothing without addons.

I'm not sure I agree with this, but the main driver is that any system should have a story for the simplest possible use case. If "plugin everything" is the direction to go, addons should be simplified and taught progressively, starting from a single file, and that's what the blueprints should output also. Even so, I think the final developer experience would look like this, making it possible to load and run build time code without understanding the whole system.

I completely agree. ember-cli has traditionally been underdocumented and has too much Ember specific stuff in it. I'm not sure anyone has ever written a decent tutorial for using ember-cli outside of Ember. There are no blueprints for that use case. The hooks have never been perfect, so FastBoot, Engines, and Module Unification specific code has ended up in ember-cli. That original vision has not been kept.

[bendemboski]

Yes, love it, totally in support of it. One question that I think needs answering has to do with the ordering. Addons can use before and after entries in the ember-addon section of package.json to ensure they run before/after other addons when there are ordering dependencies (which I think is somewhat uncommon, but definitely a used and IMO necessary configuration option).

So, my questions are:

1. When do the app's hooks run relative to its addons' hooks? Before? After?
2. Do we allow the app to customize this with something equivalent to addons' before/after values?

I don't really have a good idea of the answer to (1) -- probably depends on what we think are the most common use cases for this functionality.

For (2) I think I'd propose that we just add afterAddons and beforeAddons keys to the options passed to the EmberApp class that have exactly the same effect as the corresponding addon config values.

[rwjblue]

Hmm, this doesn't seem quite correct. You can customize the default trees by passing options to new EmberApp() like this:

let app = new EmberApp(defaults, {
  trees: {
    app: buildCustomAppTree(),
  }
});

The default implementation could be thought of as:

let app = new EmberApp(defaults, {
  trees: {
    app: 'app',
    tests: 'tests',
    vendor: fs.existsSync('vendor') ? 'vendor' : null,
  }
});

[Gaurav0]

Whlle this is true, it is not, to my knowledge, documented anywhere and may not even be public api.

[mehulkar]

Thanks for the pointer, @rwjblue, I didn't know about that. Reference for future: https://github.com/ember-cli/ember-cli/blob/v3.18.0/lib/broccoli/ember-app.js#L319-L327

The proposal is to add additional trees, similar to what you do with addons, rather than having to start from scratch with a completely custom trees.

[rwjblue]

I don't fully understand the proposal here, can you update this section to add more details about the design being proposed?

Are you suggesting adding something like:

let app = new EmberApp(defaults, {
  treeFor(type, tree) {
    if (type === 'app') {
      return myCustomAppTree();
    }

    return tree;
  }
});

?

[mehulkar]

I did some more code reading and it looks like treeFor and treeFor<Type> are pretty different actually. The former is a blank slate, the latter receives the "implicit tree" (my name) from the addon's directory structure. That kind of makes it like a post processing hook.

I've updated the design section to explicitly say that we should add the treeFor<Type> variants and what it should receive as an argument.

[mehulkar]

@bendemboski I've updated to answer your questions. I think the app variants of the hooks should run after addon hooks. Changing that order would require passing in a totally custom tree to new EmberApp.

[mehulkar]

Could others from the CLI team weigh in as well? @kategengler @kellyselden @kratiahuja @twokul

Tagging @ef4 as well since embroider is a big part of the equation in CLI world right now.

[ef4]

This RFC is in direct conflict with the embroider RFC. It would move Ember in exactly the opposite direction, toward more places where arbitrary code can generate arbitrary code.

I want to emphasize strongly why the embroider RFC is structured as a package format specification and not a build pipeline description: you get portability and stability by specifying what, not how. This RFC is entirely in terms of "how" and not "what".

I would be happy to look over the underlying use cases for which you wish you had these hooks. The intention is that all of them should be doable using the macro system, and that should be much easier for developers to read and write than being force to drop down to the broccoli level.

[mehulkar]

@ef4 that's great to know, thanks for chiming in. In the most simplistic terms, my position is that many ember addons should not need to exist, because they simply call a function at the right time. There is no need to ship a package that does this, especially if the function is also shipped in its own package.

Just to make sure we're on the same page, would you agree that today, "describing what" is not a reality, because addons can also only describe the "how", when it comes to the treeFor hooks. They do describe the "what" for ember components/services/etc, but any custom build steps happen by tapping into the broccoli tree. In the Embroider world, will a package be able to describe, for example, create a JSON file at build time? To be clear, personally I don't really care for broccoli, so I'm not married to this solution; the intention of this RFC is just to build on what exists today.

In any case, the salient question regarding the conflict here comes back to "when is embroider actually going to be viable?" Nobody can/wants to commit to an answer, of course, but under this light, is there any incremental progress we can make while we wait?

[jrjohnson]

The two use cases I would want this for are in https://github.com/ilios/frontend/tree/master/lib as in-repo addons:

1. ilios-error catches pre-boot ember errors. Specifically cases where the users browser is not suported by our build targets. In that case it uses the build targets to create a list of the browsers we DO support and sends a nice message to the user so they can identify the issue.

2. ilios-loading is static content that needs to make it into index.html during the build. I don't want to add it to index.html directly because then blueprint changes are harder to see when we update.

I assume these keep working with in-repo addons in embroiderer? By the entire in-repo addon setup is kind of a weird edge case that doesn't seem to fit. Maybe embroider can do this another way? My understanding of the process is entirely at the addon level (at best).

[ef4]

In the Embroider world, will a package be able to describe, for example, create a JSON file at build time?

Yes, this is exactly what the macro system is for.

"when is embroider actually going to be viable?"

@embroider/macros -- the part of Embroider that replaces custom broccoli hooks -- is already supported in the current stable Ember release, because it's implemented as an addon and works correctly under both the stock ember-cli build pipeline and the full Embroider build pipeline.

Now, I'm not trying to claim it's "done", because we still need better documentation and teaching materials so people can see how to plug the pieces together. But the pieces have been written and heavily tested for quite a while already. This is not far-future capability, this is today's capability.

[ef4]

@jrjohnson both of your examples use contentFor, so they wouldn't be covered by this RFC, which I don't think allows apps to do contentFor, only treeFor.

Embroider doesn't have a super strong opinion about contentFor, it's the custom broccoli hooks that are problematic.

Your custom treeForPublic in ilios-error exists to generate code that knows the list of supported browsers. That is a great use case for the macro system. As for running pre-boot, that distinction only really matters today because of the inflexibility of the current build pipeline. Under embroider you can put your own <script type="module"> tag before all the other tags in index.html and put the error handling code in there, and (critically) your error handling code would still go through the standard build (modules, babel, macros, etc). Today in treeForPublic people are entirely on their own, outside of all that stuff.

[mehulkar]

@embroider/macros -- the part of Embroider that replaces custom broccoli hooks -- is already supported in the current stable Ember release, because it's implemented as an addon and works correctly under both the stock ember-cli build pipeline and the full Embroider build pipeline

Oh interesting. So are these the appropriate next steps?

1. RFC to deprecate treeFor hooks
2. Ember CLI guides for using @embroider/macros

(Could the rest of the CLI core team weight in on this also?)

[ef4]

Deprecation is premature. It would generate a ton of noise for app authors that isn’t actionable. I think the next step is getting the resources (docs, examples, and supporting utilities) for addon authors into a complete state. That will let them begin doing things the new way.

…

On Fri, Jun 12, 2020 at 4:57 PM Mehul Kar ***@***.***> wrote: @embroider/macros -- the part of Embroider that replaces custom broccoli hooks -- is already supported in the current stable Ember release, because it's implemented as an addon and works correctly under both the stock ember-cli build pipeline and the full Embroider build pipeline Oh interesting. So are these the appropriate next steps? 1. RFC to deprecate treeFor hooks 2. Ember CLI guides for using @embroider/macros Could the rest of the CLI core team weight in on that? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#578 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACN6MUHHA7WVWE7MHGC453RWKJERANCNFSM4KFN6FEQ> .

[mehulkar]

I think the next step is getting the resources (docs, examples, and
supporting utilities) for addon authors into a complete state.

@jenweber @locks @amyrlam @mansona (as well as others from learning team), would you be open to adding a section to the CLI guides about this? (and will that require another RFC?) I can do the research and write the docs.

[kategengler]

I don't think writing guides for the macro system would require a new RFC since the system is described in https://github.com/emberjs/rfcs/blob/master/text/0507-embroider-v2-package-format.md

[ef4]

The above discussion opens several action items for alternative solution to this RFC and embroider is far more mature now than when this was opened, so I'm closing.