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

Add static asset loader for fonts and images. #1323

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

NullVoxPopuli
Copy link
Collaborator

@NullVoxPopuli NullVoxPopuli commented Jan 10, 2023

People don't like configuring webpack (this is known, across all ecosystems, not just ember).

Today, we support importing CSS in JS, like :

import 'some-path/to-a.css'

for example, which mostly does the right thing, and adds the CSS file to your module graph (doesn't make the CSS available in JS for use class-use in JS, etc)

However, this file also has fonts in it.

ember-auto-import and embroider don't currently have a font loader.

In this PR, I've added a config while working with @spuxx1701, which I believe is "just from around the internet".

It brings up some questions:

  • are there different ways we want to handle image formats?
    • low-quality-image-placeholders?
    • size optimizations?
  • are there different ways we want to handle SVG?
    • <use>
    • as components?
  • for fonts, do similar questions about technique exist that we maybe can't solve for everyone?
    • if not, should this PR only focus on fonts, since images and SVGs can be handled at least 5 ways each?

@SergeAstapov
Copy link
Collaborator

SergeAstapov commented Jan 10, 2023

2 cents for 1 and 2, just IMO:
we shouldn't do anything fancy in Embroider itself and simply make files available "as is".
any optimisations, different strategies should be implemented in packages like ember-svg-jar or/and ember-responsive-image

@NullVoxPopuli
Copy link
Collaborator Author

NullVoxPopuli commented Jan 10, 2023

ember-svg-jar and ember-responsive-image would need to provide either webpack plugins are unplugins -- which still need webpack / etc configuration -- they cannot do anything automatically -- but they could document how to do this configuration for both ember-auto-import and embroider.
But! you are right -- these things needed by these addons would likely conflict with this PR's changes as-is.

So, we should only do fonts?

@SergeAstapov
Copy link
Collaborator

maybe we could do images but e.g. in a way that is easily "swappable" with ember-svg-jar or/and ember-responsive-image ?

@NullVoxPopuli
Copy link
Collaborator Author

that would be ideal for config-resistant folks, I think -- but I'm not sure how it'd work, technically

@ef4
Copy link
Contributor

ef4 commented Jan 10, 2023

Next step here is to keep exploring / pushing-forward this kind of RFC: emberjs/rfcs#763

@simonihmig
Copy link
Collaborator

FWIW, the next-gen v2 version of ember-responsive-image will ship with custom webpack loaders, but it will also suggest users to add a custom module rule like this that will only activate its loaders when a certain query param is in the module specifier, so users can opt into the responsive-way of importing images while still keeping any other image imports the same as they are (the "virtual" module that these loaders emit are vastly different and incompatible, so using responsive loader for all image imports would break things).

So I think the default for imports of any static assets should be what the RFC suggests, to return the URL of that asset. If people want more, they can opt into that by adding custom loaders.

test: /\.(jpg|jpeg|png|woff|woff2|eot|ttf|svg)$/,
use: [
{
loader: 'url-loader',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is deprecated and replaced by asset loader for webpack 5!

@allthesignals
Copy link

I think there should at least be some mention of this in the readme. Maybe a link to some standard configuration like fonts/images?

Personally, I am not familiar with Webpack (but I should be!). Using Ember, I expect a "batteries included" experience that I've had in the past. Limiting boilerplate, etc.

Today, I was using Tailwind to try something out, and this code broke my build:

<div class="bg-[url('/sample.png')]"></div>

Not knowing much about Webpack, I was surprised to see it even trying to do anything with sample.png:

webpack 5.76.3 compiled with 1 error in 5311 ms
Build Error (PackagerRunner) in sample.png

Module parse failed: Unexpected character '�' (1:0)

But, it finally made sense when I realized that I was importing CSS directly inside app.js! Okay, something I've done in React projects, like Gatbsy, where there's configuration that tells Webpack how to handle images.

I imagine that that asset RFC will tackle things like this. But maybe just adding a final step in the readme will prevent some confusion.

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

Successfully merging this pull request may close these issues.

5 participants