Skip to content

[AssetMapper] Flexible public paths + relative path imports + possibility of "building" assets #50264

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

Merged
merged 0 commits into from
May 12, 2023

Conversation

weaverryan
Copy link
Member

Q A
Branch? 6.3
Bug fix? no
New feature? yes
Deprecations? no
Tickets None
License MIT
Doc PR Still a TODO in general

Hi!

Working hard to see where AssetMapper adjustments need to be made. 3 changes proposed here:

  1. AssetMapper's "public output path" awareness was moved to a new PublicAssetsPathResolver service
  2. The asset compilers were updated to use relative paths when adjusting paths in files.
  3. Drop the "mime type" stuff entirely - except for the "dev server" so it can set the content-type. Asset compiler's can use $mappedAsset->getPublicExtension() instead.

Why these changes?

  1. Allow public/assets/ structure to flatten/change

The new PublicAssetsPathResolver + the "adjustment" of import/url paths in the compilers allows the "output" directory structure to be decoupled from the source structure. Currently the output structure matches your "source" structure, so you get things like public/assets/styles/app.css or public/assets/bundles/easyadmin/foo.css. We could add an option in the future (or the user could decorate PublicAssetsPathResolver immediately) to "flatten" everything into just public/assets/.

  1. Allow for building of certain files, like .scss or .jsx

These changes allow users to create... more controversial functionality that could build certain files automatically. For example, you could have:

<link rel="stylesheet" href="{{ asset('styles/app.scss') }}">

Locally, I have a custom compiler that executes the dart-sass binary to convert this to CSS. Then, by decorating PublicAssetsPathResolver, I'm changing the public path for styles/app.scss so that it ends with .css. This is about 50 lines of code that results in the sass-compiled CSS file being properly rendered onto the page.

I have a similar compiler + path decorator locally that compiles .jsx => .js using the babel binary. Usage:

// assets/app.js
import HelloWorldComponent from './HelloReact.jsx';

// ... render like normal

I realize usage like this is more controversial... however it's also super pragmatic if you DO use Sass or have some React components. The alternative would be to run the sass binary manually, import the built .css file and probably ignore that file from git.

Thanks for your consideration :).

Cheers!

Copy link
Member

@javiereguiluz javiereguiluz left a comment

Choose a reason for hiding this comment

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

After thinking about this, I changed my initial thoughts and now I'm 👍 about this.

It's true that it's controversial and the "you are recreating Assetic" argument is half-true .... but at the same time:

  • Without any utility to build SCSS into CSS, I can't use AssetMapper. I'd stick to Webpack, Encore, node.js, etc.
  • My current usage of SCSS is minimal (and from what I've seen, for many other Symfony developers is the same)
  • In fact, my only current reason for using SCSS is selector nesting. This is already a native browser feature (https://caniuse.com/css-nesting) so, as soon as there's more support for it, I'll remove SCSS and just use plain CSS.

So, this "controversial" feature would be useful for me during the 1 or 2 year transition from today until I can stop using SCSS and move to CSS.

Thanks Ryan!

@fabpot
Copy link
Member

fabpot commented May 12, 2023

Thank you @weaverryan.

@fabpot fabpot closed this May 12, 2023
@fabpot fabpot force-pushed the asset-mapper-public-path-resolving branch from 58830ca to 53e297d Compare May 12, 2023 07:10
@fabpot fabpot merged commit e3a816d into symfony:6.3 May 12, 2023
@weaverryan weaverryan deleted the asset-mapper-public-path-resolving branch May 12, 2023 09:44
fabpot added a commit that referenced this pull request May 13, 2023
This PR was squashed before being merged into the 6.3 branch.

Discussion
----------

[AssetMapper] Add cached asset factory

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | None
| License       | MIT
| Doc PR        | Still TODO

Hi!

This is built on top of #50264. I CAN separate them if needed - sorry for the noise.

tl;dr When using asset mapper, to load the page, we need to build the importmap & resolve a few paths, like `{{ asset('styles/app.css') }}`. Because asset mapper loads the contents and performs some regex on CSS and JS files, this can slow down the page in dev (in prod, `asset-map:compile` removes all overhead).

This PR fixes that (I can see a big improvement in my test app). We "cache" the `MappedAsset` objects via the config cache system so that if an underlying file changes (e.g. `assets/styles/app.css`), we only need to rebuild that ONE `MappedAsset` when the page is loading.

Cheers!

Commits
-------

ce77ed8 [AssetMapper] Add cached asset factory
@fabpot fabpot mentioned this pull request May 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants