2.x not compatible with broccoli-asset-rev prepend

[jamescdavis]

We use broccoli-asset-rev's prepend option to prepend assets with the CDN we use to serve them. The chunks generated by ember-auto-import 2.x are not being prefixed (which is possibly because ember-auto-import 2.x is configured to run before broccoli-asset-rev runs, which I'm guessing is because there are issues running it post-fingerprinting). Without the CDN prefix prepended, the browser is unable to find the chunks (the "regular" asset files are prepended with the CDN as expected). Is there something we can do to work around this?

[rwjblue]

#409 is not the same thing, but seems somewhat related (and might provide more context)

[jamescdavis]

There's also #381, which is also a different (but related) issue.

[jamescdavis]

I'll note that using broccoli-asset-rev prepend is taught in the CLI guides, so seems pretty important to support. 🙂 It's also mentioned here: https://cli.emberjs.com/release/basic-use/deploying/#configuringrooturl

[jamescdavis]

Also, I'm happy to put some time in to help implement a solution. I just opened this for visibility and to start the discussion. 🙂

[ef4]

I'll note that using broccoli-asset-rev prepend is taught in the CLI guides, so seems pretty important to support.

While I do agree with this, expect to see that part of the guides get rewritten to remove all mention of broccoli-asset-rev as soon as we get consensus to make embroider the default.

But yes, ember-auto-import is supposed to work with ember as it is today and needs to play nice with broccoli-asset-rev.

[ef4]

I think #381 is the issue we should fix, and then people should set pubicAssetURL. That is the reliable way to get both eager and lazy bundles loading from an arbitrary URL prefix.

prepend isn't going to be able to do that correctly. It might get the eager ones, but it won't be able to tell webpack's runtime loader how to adjust the paths it's generating for the lazy ones.

[jamescdavis]

That makes sense to me, but is there any way to automatically set publicAssetUrl from fingerprint.prepend?

[ef4]

Maybe. But I want everybody to stop using broccoli-asset-rev, because it's unfixably unsound. So making config automatically flow from broccoli-asset-rev to ember-auto-import creates a refactor hazard later.

[jamescdavis]

Agreed. I don't love the idea I proposed, but it's going to take a while to get everyone off of broccoli-asset-rev. As long as there is a workable alternative and good guidance on how to migrate, I'm on board with not supporting broccoli-asset-rev.

[ctcpip]

since we're talking about fingerprinting / prepend here with broc asset rev, just throwing out this old unresolved issue in case it's relevant for this discussion in any way: ember-cli/ember-cli#7157

[timiyay]

Couldn't fingerprint.prepend be considered part of the public API of ember-cli, as opposed to anything specific to broccoli-asset-rev?

And, therefore, the implementation could be changed to pass this ember-cli config to webpack, instead of broccoli-asset-rev, and most devs wouldn't notice if the end result stays the same?

As a longtime Ember dev POV, the only reason we're using broccoli-asset-rev is that ember-cli requires us to do so, and the ember-cli blueprints for package.json make it a direct dependency required by Ember apps. It has never been a conscious choice to pull broccoli-asset-rev into our apps.

It may depend on how embroider devs perceive it being rolled out. Is it going to cause breaking changes in ember-cli, forcing a bump to a new major version? How will that major version bump play with the practice of synchronising with the ember-source version, which is approaching a new major version of its own?

We're trying to get embroider going, but it appears like it'll be non-trivial, breaking churn, alongside the same churn for Octaneifying our legacy apps for Ember 4.0, so I'm trying to get a feel for how we're going to schedule these 2 large pieces of work.

[ef4]

@timiyay I think it's important to distinguish different kinds of upgrade costs.

If embroider or ember-auto-import 2.0 required you to audit your whole app and touch many files, that would be unreasonable because the cost is unbounded and grows with the size of your app.

If we required all your addons to do something first, that would also be unreasonable, because then you're blocked by code outside your control, and the cost grows with the number of addons and their interactions.

But if we require you to make small changes to ember-cli-build.js in order to get access to a new feature like embroider or ember-auto-import 2.0, that's an acceptable cost. It's bounded. No matter how big your app is, the upgrade step is the same. And it's not a breaking change, because it's opt in. There is no plan to deprecate the non-embroider build.

Couldn't fingerprint.prepend be considered part of the public API of ember-cli, as opposed to anything specific to broccoli-asset-rev?

It could, but that is exactly the problem I'm trying to solve here. It's a problem that the division of responsibility and consequent stability is so murky in this area.

It's a problem that people think broccoli-asset-rev is required, because it's not. It's a problem that the guides tell you how to use fingerprint.prepend without explaining that it's really a feature of broccoli-asset-rev.

broccoli-asset-rev got into the default blueprint before we had a process for ensuring stability and consensus. It couldn't pass that bar today, because it has multiple fatal problems. Its implementation is unsafe, it doesn't work with arbitrary lazy loading, and even its name is incorrectly focused on its implementation and not its interface.

It's totally understandable that Ember devs use it and trust it because that's what the signals in the blueprints and docs are telling them. But that's why I'm being a stickler here: I want to send a different signal.

Is it going to cause breaking changes in ember-cli, forcing a bump to a new major version?

No, there's no major break to use embroider. It only effects you if you turn it on. Embroider supports ember-cli back to 3.16 and forward into the upcoming 4.x.

[timiyay]

@ef4 Thanks for the explanation, that helps me to set expectations (my own and my colleagues) for issues like this, as we consider the jump to embroider.

[ef4]

I started working on this by enabling prepend in one of ember-auto-import's test apps and broccoli-asset-rev immediately injected a syntax error that kills the whole app. 🤣 This is what I mean when I say it's not safe.

I know I should ship a replacement instead of just complaining, but the thing is, once you're on embroider you don't need a replacement. Asset URL handling is built into webpack 5 and any other bundler you'd want to plug into embroider.

Perhaps this is another feature we should just polyfill with ember-auto-import, so apps can start moving toward better patterns. 🤔

[ef4]

This is fixed in #415, because publicAssetURL now controls these entry chunks. And I added a section to the 2.0 upgrade guide: https://github.com/ef4/ember-auto-import/blob/56cdf5b0fca3b9513c84f52b7bbbd2621b23796e/docs/upgrade-guide-2.0.md#publicasseturl

[rsmith-cs-ux]

I'm having an issue related to this (I believe). In my app I have a prepend AND publicAssetURL defined just like the docs recommend but I'm getting duplicate urls when running the build. Should we be using one or the other but not both? Or is there another issue here that I should file?

[ef4]

Yes, please file an issue if you're having trouble.