rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

Asset Import Spec

Open ef4 opened this issue 3 years ago • 5 comments

Rendered

ef4 avatar Aug 13 '21 20:08 ef4

Rendered (post file rename)

NullVoxPopuli avatar Aug 13 '21 20:08 NullVoxPopuli

How would importing assets from CSS work? eg images and fonts?

chancancode avatar Aug 13 '21 21:08 chancancode

Do I understand correctly, that if I'd want dynamic filenames I'd have to continue using /public? E.g. something like this:

export default class FlagComponent  extends Component {
  get url() {
    return `/assets/flags/${this.args.country}.png`;
  }
}

I know that also does not "just work" with the current broccoli-asset-rev fingerprinting, but might be interesting to mention how dynamic asset names would/should work under the new format (which I am generally def. in favor of!). Depending on the use case, I guess either an exhaustive map (if you don't have too many options) would work fine, but for larger lists of options (e.g. country flags) it might be a bit "overkill":

import flagDe from 'my-app/flags/de.png';
import flagAt from 'my-app/flags/at.png';
import flagUk from 'my-app/flags/uk.png';

const flagMap = {
  de: flagDe,
  at: flagAt,
  uk: flagUk
};

export default class FlagComponent extends Component {
  get url() {
    return flagMap[this.args.country];
  }
}

mydea avatar Aug 16 '21 07:08 mydea

How would importing assets from CSS work? eg images and fonts?

I should be more explicit about it in the text, but what I meant by:

From Type To Type Example Syntax Semantics
CSS CSS @import("thing/style.css") W3C plus NodeJS resolving rules
CSS Other url("thing/icon.png"); W3C plus NodeJS resolving rules

is that we promise that those syntaxes will obey their original W3C semantics, with the addition of the ability to do node-style package resolution (so that the imported paths have a consistent meaning between here and in JS).

Like with JS, how we actually implement the semantics is left unspecified. @import for example is totally allowed to be inlined, or not. url() could end up containing a data URI, etc.

Providing the option to fine-tune the implementation is left up to each specific bundler. In a classic build, that would be ember-auto-import and its webpack.

ef4 avatar Aug 18 '21 01:08 ef4

What's the status on this? Should we get more formal review?

wagenet avatar Jul 23 '22 23:07 wagenet

Context: @kategengler has some updates to the RFC here: https://github.com/emberjs/rfcs/pull/973

achambers avatar Oct 06 '23 14:10 achambers

I have merged updates from @kategengler and taken this out of draft, please take a look again as we think it's ready to advance.

ef4 avatar Oct 13 '23 18:10 ef4

On the implementation side, how does this RFC relate to https://github.com/embroider-build/ember-auto-import/issues/565? My understanding would be, that a good part of this RFC's implementation is to get that eai feature to land, the other part is https://github.com/webpack/webpack/issues/16693 and some additional default webpack config to set up the asset loader, is that fair to say?

On the eai feature, as it is currently envisaged, it would require users to opt-in by specifying a pattern for all the stuff that eai should handle, that would have traditionally been part of the classic build. But it seems according to the RFC importing anything (that's not a module or CSS I guess?) is supposed to "just work", correct?

simonihmig avatar Oct 25 '23 17:10 simonihmig

@simonihmig this RFC no longer proposes using import statements for assets. It proposes import.meta.resolve().

The implementation of that feature in classic apps might very well be handled by ember-auto-import, but it would not require any config if we decide to land this RFC and make it standard.

ef4 avatar Oct 27 '23 17:10 ef4

@ef4 I understand the recent changes in this RFC. But I still feel there is a bit of overlap between this RFC and the proposed eai feature, that could lead to some inconsistencies...

For example, under this RFC you would be able to import.meta.resolve('./style.css') if you wanted to, right? If you don't want just the URL, but the side-effect of applying the CSS, you could do import './style.css', but only for v2 addons and Embroider apps. For v1 addons or classic apps, that would be possible with that eai feature, but as it stands only with some additional configuration.

For a more complete map of which kind of asset handling is possible:

Expression v1 addon v2 addon classic app Embroider app
import.meta.resolve('./logo.png') 🟢 🟢 🟢 🟢
import.meta.resolve('./style.css') 🟢 🟢 🟢 🟢
import logo from './logo.png' [^1] 🟠🟡 🟡 🟠🟡 🟡
import './style.css' 🟠 🟢 🟠 🟢
import.meta.resolve('./data.yml') 🟢 🟢 🟢 🟢
import parsedData from './data.yml' 🟠🟡 🟡 🟠🟡 🟡
[^1]: this example is superseded by this RFC, but still possible to set up

🟢 works out of the box, according to this RFC / RFC507 (v2 spec) 🟡 needs custom webpack loader configuration 🟠 needs https://github.com/embroider-build/ember-auto-import/issues/565 and custom config

The 🟡 part of needing custom config for importing unexpected file types is expected, and not my point here. But my point is about the 🟠 part of being able to import your own assets, where v1 addons/classic apps need that eai feature with a custom config (which btw is not backed by an RFC), while this "just works" for v2 addons/Embroider apps.

This RFC makes import.meta.resolve() work symmetrically across all types of addons/apps. Shouldn't we strive for similar symmetry for the other cases?

Implementation-wise, I would think this shouldn't be a big problem when we have that eai feature, given that it was never possible to import anything but JavaScript for v1 addons and classic apps, so requiring a config is not really need. We could just enable eai to handle any (local, relative) import without any explicit config, when that import refers to anything that is not JavaScript, couldn't we? Which would make the table above symmetric again.

Does that make sense? And if so, should this be a short follow-up RFC?

simonihmig avatar Oct 31 '23 21:10 simonihmig