feat(dev): added assets to manifest #6649

lubomirblazekcz · 2022-01-26T19:14:11Z

Description

Resolves #6595 and #2375 #4198

Additional context

All assets are now included in manifest as separate entry

What is the purpose of this pull request?

Bug fix
New Feature
Documentation update
Other

Before submitting the PR, please make sure you do the following

Read the Contributing Guidelines.
Read the Pull Request Guidelines and follow the Commit Convention.
Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
Ideally, include relevant tests that fail without this PR but pass with it.

lubomirblazekcz · 2022-01-26T20:37:49Z

I would need some help solving the second part of issue #6595 if entry file is png, jpg, svg, etc. then the chunks are generated as empty .js. I didn't find where could I change this.

This doesn't happend with .css, other static assets should also copy to dist if included as input entrypoint. It's important for backend integration, beucase there can be static assets paths in templates (.twig, .blade, .latte, etc.) and there is no way to reslove them on backend if they are not processed correctly and part of the manifest.

Generated an empty chunk: "fragment.svg"
Generated an empty chunk: "icon.png"

innocenzi · 2022-03-09T18:38:59Z

It'd be nice if you could add a test for this feature: https://github.com/lubomirblazekcz/vite/blob/manifest-assets/packages/playground/backend-integration/__tests__/backend-integration.spec.ts#L27

If the second part is a blocker, maybe you can modify the PR to support only CSS for now, and then PR again with a fix for other assets? Just to get things moving in that direction.

In multiple back-end adapters, we had to add a plugin that transforms the manifest to support finding CSS files, so this would be really nice to have.

innocenzi · 2022-03-09T18:42:14Z

packages/vite/src/node/plugins/manifest.ts

@@ -101,10 +101,18 @@ export function manifestPlugin(config: ResolvedConfig): Plugin {
        return manifestChunk
      }

+      function createAsset(chunk: OutputAsset): ManifestChunk {
+        return {
+          file: chunk.fileName


Having src here if it exists would be helpful too.

Wouldn't be the src same as the name of the asset? Also not sure how to do this, asset returns only name

if (chunk.name) { manifestAsset.src = chunk.name }

Would this be correct? I don't know how to get full path for assets, they don't include facadeModuleId

…ssets

lubomirblazekcz · 2022-03-15T14:57:31Z

Hi @innocenzi I've added the tests here https://github.com/vitejs/vite/pull/6649/files#diff-f21fb4f1a20cc108f9e9d6c1f086b7d74bf973bf2646e73d2087a8ec28a1e5ad

The double css is there because of the vite config, could be resolved with something like this (line 19). Not sure if it's the best way or necessary though.

.map((filename) => [path.relative(root, filename).replace('.css', ''), filename])

Also the second part is not blocker, it's still beneficial to have all assets in manifest. But it would be great if static assets could be added as entrypoint, but as I said - not sure how to fix that.

# Conflicts: # packages/playground/backend-integration/__tests__/backend-integration.spec.ts

lubomirblazekcz · 2022-05-28T15:10:05Z

@innocenzi @patak-dev any chance getting this into v3? I think this would also solve #2375 and #4198

innocenzi · 2022-05-28T17:55:49Z

Would love this PR to be merged, but I think it's not completely done yet?

I would need some help solving the second part of issue #6595 if entry file is png, jpg, svg, etc. then the chunks are generated as empty .js. I didn't find where could I change this.

lubomirblazekcz · 2022-05-28T18:07:45Z

That is the current behaviour in vite now and it's not something this PR resolves. I will split the issue, so it's more clear.

This PR resolves that CSS files and other (non entry assets) are generated into manifest as separate entries.

lubomirblazekcz · 2022-05-28T18:16:35Z

@innocenzi I've changed the description of my issue and this PR, so it's clear what it resolves 👍

Also created a new issue regarding the problem I‘ve described in that previous comment - #8373

jessarcher · 2022-06-04T01:29:40Z

Thanks for this PR!

I'm concerned about chunk.name just being the file name and not the full path to the source asset.

Given the following entry points:

build: {
  rollupOptions: {
    input: [
      'resources/js/app.js',
      'resources/css/app.css',
    ],
  },
}

The manifest would end up as:

{
  "resources/js/app.js": {
    "file": "assets/app.21787142.js",
    "src": "resources/js/app.js",
    "isEntry": true
  },
  "app.css": {
    "file": "assets/app.2c05110b.css",
    "src": "app.css"
  }
}

Without the full path, it's not really possible to correctly map from the source file to the compiled asset (unless you can trust there is only one file with that name). It's also inconsistent with the JS chunks and allows for naming collisions.

I need to spend a bit more time digging through Rollup, but I'm worried this can't be resolved without an upstream change.

lubomirblazekcz · 2022-06-04T08:39:13Z

@jessarcher in my tests chunk.name returns whole path relative to entrypoint. At least it returns the path same way as for JS. Also filename collisions shoudn't occur, because the path is relative to entrypoint.

For reference, this is from my tests in playground/backend-integration

rollupOptions: {
   input: [
     'frontend/entrypoints/main.ts',
     'frontend/entrypoints/global.css'
   ]
 }

{
  "../images/logo.png": {
    "file": "assets/logo.b9f46fb3.png",
    "src": "../images/logo.png"
  },
  "main.ts": {
    "file": "assets/main.ad17e0e9.js",
    "src": "main.ts",
    "isEntry": true
  },
  "global.css": {
    "file": "assets/global.36c3a1e4.css",
    "src": "global.css"
  }
}

jessarcher · 2022-06-08T02:56:02Z

@lubomirblazekcz I think that's because in the test, root has been specified as frontend/entrypoints.

If the root was not a subdirectory, then main.ts would be frontend/entrypoints/main.ts, but global.css would remain global.css.

This is because for the "chunk" type, it uses getChunkName(), which looks at chunk.facadeModuleId rather than chunk.name. Unfortunately, chunk.facadeModuleId doesn't exist on "asset" chunks.

In css.ts, the full CSS chunk is replaced in the bundle with an asset chunk.

In userland, the only way I can find to get the path is by grabbing it in the renderChunk hook where the original chunk is still present with the facadeModuleId.

In Vite itself, I'm exploring a few options, such as keeping the original CSS chunk if it's an entrypoint, but I'm not sure on the impacts.

The tests are a little funny because global.css is an entrypoint, but it's also being imported in index.html, so I might try to tweak the tests a bit to separate the CSS entry point from the imported CSS.

jessarcher · 2022-06-08T14:28:28Z

I've tried making the following change in css.ts:

- let isPureCssChunk = true
+ let isPureCssChunk = !chunk.isEntry

This prevents the CSS chunk from being removed from the bundle so it can be added to the manifest, however, it doesn't have the desired result (via playground/backend-integration).

Before:

{
  'index.html': {
    file: 'assets/index.html.6a442648.js',
    src: 'index.html',
    isEntry: true,
    imports: [ 'main.ts' ],
    css: [ 'assets/global.css.d99fc02a.css' ],
    assets: [ 'assets/logo.b9f46fb3.png' ]
  },
  // ...
}

After:

{
  'global.css': {
    file: 'assets/global.css.9f516e50.js',
    src: 'global.css',
    isEntry: true,
    css: [ 'assets/global.css.d99fc02a.css' ],
    assets: [ 'assets/logo.b9f46fb3.png' ]
  },
  'index.html': {
    file: 'assets/index.html.6a442648.js',
    src: 'index.html',
    isEntry: true,
    imports: [ 'main.ts', 'global.css' ],
    assets: [ 'assets/logo.b9f46fb3.png' ]
  },
  // ...
}

The CSS entry point gets correctly added to the manifest (and will have a path where appropriate) however the file is a JS file (?!). Additionally, index.html loses its css key, with the CSS being moved to the imports key instead. The latter may not be an issue in practice because it seems weird to import a CSS file that's also an entry point.

When the CSS chunk is removed from the bundle, there doesn't seem to be any way to get its path correctly. I did notice that removed CSS chunks are stored in removedPureCssFilesCache so I could import that in manifest.ts to append to the manifest, but that seems like a hack.

lubomirblazekcz · 2022-06-08T15:07:31Z

@jessarcher I see now, thanks for clearing it out. Seems this problem is only for css files. I will try to look into this. If you found suitable solution yourself I can add you to my forked project so you could add it in this PR.

jessarcher · 2022-06-08T16:08:39Z

@lubomirblazekcz Oh, I didn't notice it was only a problem with CSS!

After looking at asset.ts, something like this in css.ts seems to solve it when combined with your PR!

  const fileHandle = this.emitFile({
-   name: cssAssetName,
+   name: chunk.facadeModuleId
+     ? normalizePath(path.relative(config.root, chunk.facadeModuleId))
+     : cssAssetName,
    type: 'asset',
    source: chunkCSS
})

I have no idea if it would be accepted, but feel free to add it or give me access 🙂

lubomirblazekcz · 2022-06-09T08:35:09Z

@jessarcher thanks that indeed fixed the issue! But it broke a bunch of tests, I will have to investigate further what went wrong :/

lubomirblazekcz · 2022-06-09T08:51:19Z

It broke playground/assets, for some reason index.html is now treated as asset and not entry, so it doesn't include info about assets.

@jessarcher I've invited you to the repo, feel free to add any changes. I will have more time at weekend to look at this more.

jessarcher · 2022-06-10T06:15:36Z

packages/vite/src/node/plugins/css.ts

+          const output = config.build?.rollupOptions?.output
+          const assetFileNames =
+            (output && !Array.isArray(output)
+              ? output.assetFileNames
+              : undefined) ??
+            // defaults to '<assetsDir>/[name].[hash][extname]'
+            // slightly different from rollup's one ('assets/[name]-[hash][extname]')
+            path.posix.join(config.build.assetsDir, '[name].[hash][extname]')
+


This is copied verbatim from asset.ts so we could potentially extract it to function if we wanted to keep that logic and naming pattern centralised.

Done, there was also a fix for plugin legacy with custom asset file names. So this abstraction was needed for it

lubomirblazekcz · 2022-06-10T08:47:34Z

@jessarcher great work! Everything seems to work good as expected now 👍

patak-dev · 2022-06-10T10:04:35Z

Nice! Thanks for your work here. We'll discuss this PR in the next team meeting. We need to also check in the meeting if the proposal to add a version field to the manifests needs to be done as part of these changes. See #7960 (comment). I would also appreciate your input regarding it to see how this will affect future updates to the manifest and the ability of integrations to cope with them.

lubomirblazekcz · 2022-06-12T10:34:01Z

@patak-dev schema for manifests sounds like a great idea, it's something that for sure can be handy in the future

bholmesdev · 2022-06-17T14:02:09Z

This was a major blocker for Slinkity and Astro as we explore a special assets directory for SSR assets. I currently generate a virtual module to "spoof" imports just to find the input file for a given CSS output file. Assuming this gives me an input-ouput mapping for assets... it sounds like a godsend 🙏

patak-dev · 2022-06-17T22:26:14Z

We talked about the PR in the last team meeting and we're good to move forward with it. We also discussed about adding a version and we think it isn't worth completely changing the current schema to include it. We'll merge it before cutting v3 beta next Monday. Let me know if there is something more needed here. @ElMassimo pinging you as this may also be interesting for Vite Ruby.

innocenzi · 2022-06-18T10:46:59Z

This is fantastic news, thanks @patak-dev!

…m assetFileNames

ElMassimo · 2022-06-24T13:46:49Z

Currently this change causes the entries to be generated incorrectly in Vite Ruby for any Sass/Less/Stylus stylesheet, as it's erasing the original extension from the manifest:

  "blue.css": {
    "file": "assets/blue.75487b74.css",
    "src": "blue.css"
  }

In contrasst, Vite does preserve .ts extensions:

  "application.ts": {
    "file": "assets/application.48a9de7f.js",
    "src": "application.ts"
  }

For any backend integration to map files correctly, it's desirable to preserve the extension:

  "blue.scss": {
    "file": "assets/blue.12fc1eef.css",
    "src": "blue.scss"
  }

Are there any follow-up pull requests to this?

patak-dev · 2022-06-24T13:48:22Z

Not yet @ElMassimo, PR welcome, or please create an issue if not so we can track it for v3
Thanks for testing!

ElMassimo · 2022-06-24T14:02:17Z

Opened #8764 so that we don't forget, thanks! 😃

feat(dev): added assets to manifest

b3a7842

patak-dev added the p2-nice-to-have Not breaking anything but nice to have (priority) label Feb 14, 2022

innocenzi reviewed Mar 9, 2022

View reviewed changes

lubomirblazekcz added 2 commits March 15, 2022 14:15

Merge branch 'main' of https://github.com/vitejs/vite into manifest-a…

2da3765

…ssets

feat: added backend-integration assets manifest tests

80e7139

lubomirblazekcz added 4 commits May 28, 2022 16:11

Merge branch 'main' of github.com:vitejs/vite into manifest-assets

a89dd76

# Conflicts: # packages/playground/backend-integration/__tests__/backend-integration.spec.ts

fix: merge file remnant

7fd58ea

fix: lint error

5e5e970

feat: added src to assets manifest

af2ec79

patak-dev added p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority) and removed p2-nice-to-have Not breaking anything but nice to have (priority) labels Jun 3, 2022

jessarcher mentioned this pull request Jun 9, 2022

Add CSS entrypoints to the manifest laravel/vite-plugin#9

Merged

lubomirblazekcz added 2 commits June 9, 2022 10:01

fix: css asset path

22d33c8

fix(tests): css entry

1919d04

lubomirblazekcz mentioned this pull request Jun 9, 2022

RFC: Vytažení do nette extension lubomirblazekcz/nette-vite#1

Closed

jessarcher force-pushed the manifest-assets branch from b76d7e5 to 3147b88 Compare June 10, 2022 06:05

jessarcher reviewed Jun 10, 2022

View reviewed changes

patak-dev added this to the 3.0 milestone Jun 10, 2022

ScottPolhemus mentioned this pull request Jun 16, 2022

Refactor plugins and update seed-theme barrel/shopify-vite#1

Merged

patak-dev mentioned this pull request Jun 17, 2022

chore: add JSON schemas for the manifests generated by Vite #7960

Closed

10 tasks

patak-dev added 2 commits June 19, 2022 08:21

chore: merge main

1fc45de

fix: abstract resolveAssetFileNames, support legacy plugin with custo…

9773d4e

…m assetFileNames

patak-dev approved these changes Jun 19, 2022

View reviewed changes

patak-dev merged commit cdf744d into vitejs:main Jun 19, 2022

khalwat mentioned this pull request Jun 23, 2022

Allow styles to be included separately from scripts nystudio107/craft-vite#31

Closed

This was referenced Jun 24, 2022

Assets entries in the manifest do not preserve the original file extension #8764

Closed

fix: preserve extension of css assets in the manifest #8768

Merged

This was referenced Jun 26, 2022

CSS input missing from manifest.json #4198

Closed

manifest.json does not include assets #2375

Closed

khalwat mentioned this pull request Jun 29, 2022

public assets v4 nystudio107/craft-plugin-vite#9

Merged

sapphi-red mentioned this pull request Jul 1, 2022

build.cssCodeSplit: false => CSS missing in dist/manifest.json #6477

Closed

7 tasks

ElMassimo mentioned this pull request Jul 1, 2022

feat: use Vite 3 as the default ElMassimo/vite_ruby#225

Merged

khalwat mentioned this pull request Jan 25, 2023

craft.vite.asset no longer matches manifest format in Vite 3.x || 4.x nystudio107/craft-vite#56

Closed

ElMassimo mentioned this pull request May 3, 2023

Surprising output in rollup bundle sourcemaps for none .js entrypoint files ElMassimo/vite_ruby#358

Open

2 tasks

si14 mentioned this pull request Sep 3, 2023

Incorrect manifest entries for CSS entry points #14271

Closed

7 tasks

sapphi-red mentioned this pull request Nov 10, 2023

fix(css): correctly set manifest source name and emit CSS file #14945

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(dev): added assets to manifest #6649

feat(dev): added assets to manifest #6649

lubomirblazekcz commented Jan 26, 2022 •

edited

Loading

lubomirblazekcz commented Jan 26, 2022

innocenzi commented Mar 9, 2022

innocenzi Mar 9, 2022

lubomirblazekcz Mar 15, 2022

lubomirblazekcz commented Mar 15, 2022

lubomirblazekcz commented May 28, 2022

innocenzi commented May 28, 2022

lubomirblazekcz commented May 28, 2022

lubomirblazekcz commented May 28, 2022 •

edited

Loading

jessarcher commented Jun 4, 2022

lubomirblazekcz commented Jun 4, 2022

jessarcher commented Jun 8, 2022 •

edited

Loading

jessarcher commented Jun 8, 2022

lubomirblazekcz commented Jun 8, 2022

jessarcher commented Jun 8, 2022 •

edited

Loading

lubomirblazekcz commented Jun 9, 2022

lubomirblazekcz commented Jun 9, 2022

jessarcher Jun 10, 2022

patak-dev Jun 19, 2022

lubomirblazekcz commented Jun 10, 2022

patak-dev commented Jun 10, 2022

lubomirblazekcz commented Jun 12, 2022

bholmesdev commented Jun 17, 2022 •

edited

Loading

patak-dev commented Jun 17, 2022

innocenzi commented Jun 18, 2022

ElMassimo commented Jun 24, 2022

patak-dev commented Jun 24, 2022

ElMassimo commented Jun 24, 2022

feat(dev): added assets to manifest #6649

feat(dev): added assets to manifest #6649

Conversation

lubomirblazekcz commented Jan 26, 2022 • edited Loading

Description

Additional context

What is the purpose of this pull request?

Before submitting the PR, please make sure you do the following

lubomirblazekcz commented Jan 26, 2022

innocenzi commented Mar 9, 2022

innocenzi Mar 9, 2022

Choose a reason for hiding this comment

lubomirblazekcz Mar 15, 2022

Choose a reason for hiding this comment

lubomirblazekcz commented Mar 15, 2022

lubomirblazekcz commented May 28, 2022

innocenzi commented May 28, 2022

lubomirblazekcz commented May 28, 2022

lubomirblazekcz commented May 28, 2022 • edited Loading

jessarcher commented Jun 4, 2022

lubomirblazekcz commented Jun 4, 2022

jessarcher commented Jun 8, 2022 • edited Loading

jessarcher commented Jun 8, 2022

lubomirblazekcz commented Jun 8, 2022

jessarcher commented Jun 8, 2022 • edited Loading

lubomirblazekcz commented Jun 9, 2022

lubomirblazekcz commented Jun 9, 2022

jessarcher Jun 10, 2022

Choose a reason for hiding this comment

patak-dev Jun 19, 2022

Choose a reason for hiding this comment

lubomirblazekcz commented Jun 10, 2022

patak-dev commented Jun 10, 2022

lubomirblazekcz commented Jun 12, 2022

bholmesdev commented Jun 17, 2022 • edited Loading

patak-dev commented Jun 17, 2022

innocenzi commented Jun 18, 2022

ElMassimo commented Jun 24, 2022

patak-dev commented Jun 24, 2022

ElMassimo commented Jun 24, 2022

lubomirblazekcz commented Jan 26, 2022 •

edited

Loading

lubomirblazekcz commented May 28, 2022 •

edited

Loading

jessarcher commented Jun 8, 2022 •

edited

Loading

jessarcher commented Jun 8, 2022 •

edited

Loading

bholmesdev commented Jun 17, 2022 •

edited

Loading