fix(assets): ensure ?no-inline is not included in the asset url in the production environment #19496

OnlyWick · 2025-02-23T12:14:28Z

Description

close #19490.

It would be better not to retain ?no-inline in the production environment, while keeping it unchanged in the development environment.

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

noticed a bug

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

…he production environment

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

sapphi-red · 2025-02-26T06:45:46Z

while keeping it unchanged in the development environment.

What is the reason of this? For me, removing ?no-inline is feels expected as ?no-inline is just a special marker.

OnlyWick · 2025-02-26T07:14:43Z

In the development environment, the purpose of not deleting it is so that users can see if it has been added with a ?no-inline marker, which I think would be better.

sapphi-red · 2025-02-27T02:57:01Z

Doesn't that apply to the prod env as well? (I'm trying to understand the tradeoffs here)

OnlyWick · 2025-02-27T04:16:16Z

My goal is to distinguish which ones I manually added the ?no-inline marker to in the dev env. When the asset file > assetsInlineLimit, it will be directly imported using the URL. And the asset file < assetsInlineLimit will be directly converted to base64, so I add a ?no-inline to it, it will also use the URL to import. Now if we remove the ?no-inline marker, these asset files > or < assetsInlineLimit can't see any difference in the URL(If we do not view the corresponding source code during development, but view the HTML in the browser, we will not know whether the ?no-inline marker is added or not.).

For prod env, I believe that building will not go wrong, ?no-inline is a development marker, it is not necessary to appear in the bundle, and ?no-inline will also take up a few network transmission bytes.

Techn1x · 2025-04-02T02:21:05Z

When I run vite build --mode test (which I guess is technically a production build?) and run a test suite against it, the urls still have ?no-inline appended but the assets are not built at that path (they're built to the non-inlined path), so the tests 404 error when trying to find those images.

This PR fixes that for me (I've applied it as a pnpm patch for now), but wondering if anyone else hits this bug?

sapphi-red

We discussed removing ?no-inline in dev during our team meeting.
For now, we’ve decided to keep the current implementation (i.e., not removing it), as it aligns with other query formats (e.g., ?worker&url keeps ?worker_file&type=module) and likely won’t cause any issues.

Techn1x · 2025-04-23T07:10:14Z

Now that this is approved, is there a process to follow for getting this merged? (or am I just being impatient 😄 )

hi-ogawa

Thanks!

hi-ogawa · 2025-04-23T09:39:38Z

@sapphi-red Hmm I just noticed it says "feat". Can we merge this as patch fix? 🤔

sapphi-red · 2025-04-23T09:48:28Z

@hi-ogawa I think it's a fix and can be merged in a patch version 👍

OnlyWick · 2025-04-23T15:03:24Z

LOL, contributed more than 100 lines of code to Vite 😆.

Techn1x · 2025-04-24T08:45:09Z

6.3.3. works great! thanks!

| datasource | package | from | to | | ---------- | ------- | ----- | ----- | | npm | vite | 6.3.2 | 6.3.3 | ## [v6.3.3](https://github.com/vitejs/vite/blob/HEAD/packages/vite/CHANGELOG.md#small633-2025-04-24-small) - fix: ignore malformed uris in tranform middleware ([#19853](vitejs/vite#19853)) ([e4d5201](vitejs/vite@e4d5201)), closes [#19853](vitejs/vite#19853) - fix(assets): ensure ?no-inline is not included in the asset url in the production environment ([#1949](vitejs/vite#1949) ([16a73c0](vitejs/vite@16a73c0)), closes [#19496](vitejs/vite#19496) - fix(css): resolve relative imports in sass properly on Windows ([#19920](vitejs/vite#19920)) ([ffab442](vitejs/vite@ffab442)), closes [#19920](vitejs/vite#19920) - fix(deps): update all non-major dependencies ([#19899](vitejs/vite#19899)) ([a4b500e](vitejs/vite@a4b500e)), closes [#19899](vitejs/vite#19899) - fix(ssr): fix execution order of re-export ([#19841](vitejs/vite#19841)) ([ed29dee](vitejs/vite@ed29dee)), closes [#19841](vitejs/vite#19841) - fix(ssr): fix live binding of default export declaration and hoist exports getter ([#19842](vitejs/vite#19842)) ([80a91ff](vitejs/vite@80a91ff)), closes [#19842](vitejs/vite#19842) - perf: skip sourcemap generation for renderChunk hook of import-analysis-build plugin ([#19921](vitejs/vite#19921)) ([55cfd04](vitejs/vite@55cfd04)), closes [#19921](vitejs/vite#19921) - test(ssr): test `ssrTransform` re-export deps and test stacktrace with first line ([#19629](vitejs/vite#19629)) ([9399cda](vitejs/vite@9399cda)), closes [#19629](vitejs/vite#19629)

| datasource | package | from | to | | ---------- | ------- | ----- | ----- | | npm | vite | 6.3.2 | 6.3.4 | ## [v6.3.4](https://github.com/vitejs/vite/blob/HEAD/packages/vite/CHANGELOG.md#small634-2025-04-30-small) - fix: check static serve file inside sirv ([#19965](vitejs/vite#19965)) ([c22c43d](vitejs/vite@c22c43d)), closes [#19965](vitejs/vite#19965) - fix(optimizer): return plain object when using `require` to import externals in optimized dependenci ([efc5eab](vitejs/vite@efc5eab)), closes [#19940](vitejs/vite#19940) - refactor: remove duplicate plugin context type ([#19935](vitejs/vite#19935)) ([d6d01c2](vitejs/vite@d6d01c2)), closes [#19935](vitejs/vite#19935) ## [v6.3.3](https://github.com/vitejs/vite/blob/HEAD/packages/vite/CHANGELOG.md#small633-2025-04-24-small) - fix: ignore malformed uris in tranform middleware ([#19853](vitejs/vite#19853)) ([e4d5201](vitejs/vite@e4d5201)), closes [#19853](vitejs/vite#19853) - fix(assets): ensure ?no-inline is not included in the asset url in the production environment ([#1949](vitejs/vite#1949) ([16a73c0](vitejs/vite@16a73c0)), closes [#19496](vitejs/vite#19496) - fix(css): resolve relative imports in sass properly on Windows ([#19920](vitejs/vite#19920)) ([ffab442](vitejs/vite@ffab442)), closes [#19920](vitejs/vite#19920) - fix(deps): update all non-major dependencies ([#19899](vitejs/vite#19899)) ([a4b500e](vitejs/vite@a4b500e)), closes [#19899](vitejs/vite#19899) - fix(ssr): fix execution order of re-export ([#19841](vitejs/vite#19841)) ([ed29dee](vitejs/vite@ed29dee)), closes [#19841](vitejs/vite#19841) - fix(ssr): fix live binding of default export declaration and hoist exports getter ([#19842](vitejs/vite#19842)) ([80a91ff](vitejs/vite@80a91ff)), closes [#19842](vitejs/vite#19842) - perf: skip sourcemap generation for renderChunk hook of import-analysis-build plugin ([#19921](vitejs/vite#19921)) ([55cfd04](vitejs/vite@55cfd04)), closes [#19921](vitejs/vite#19921) - test(ssr): test `ssrTransform` re-export deps and test stacktrace with first line ([#19629](vitejs/vite#19629)) ([9399cda](vitejs/vite@9399cda)), closes [#19629](vitejs/vite#19629)

sapphi-red added the p2-edge-case Bug, but has workaround or limited in scope (priority) label Feb 25, 2025

sapphi-red reviewed Feb 25, 2025

View reviewed changes

packages/vite/src/node/plugins/asset.ts Outdated Show resolved Hide resolved

OnlyWick force-pushed the feat-no-inline-vite branch from 75a9e7d to dea9917 Compare February 25, 2025 05:48

This comment was marked as outdated.

Sign in to view

sapphi-red reviewed Feb 25, 2025

View reviewed changes

packages/vite/src/node/plugins/asset.ts Outdated Show resolved Hide resolved

feat(assets): ensure ?no-inline is not included in the asset url in t…

133bdc4

…he production environment

OnlyWick force-pushed the feat-no-inline-vite branch from dea9917 to 133bdc4 Compare February 25, 2025 07:00

sapphi-red reviewed Feb 26, 2025

View reviewed changes

packages/vite/src/node/plugins/asset.ts Outdated Show resolved Hide resolved

chore: merge lines

b57c6a4

sapphi-red added this to Team Board Mar 25, 2025

github-project-automation bot moved this to Discussing in Team Board Mar 25, 2025

sapphi-red moved this from Discussing to Approved in Team Board Apr 16, 2025

sapphi-red approved these changes Apr 16, 2025

View reviewed changes

sapphi-red requested a review from hi-ogawa April 23, 2025 07:37

hi-ogawa approved these changes Apr 23, 2025

View reviewed changes

sapphi-red changed the title ~~feat(assets): ensure ?no-inline is not included in the asset url in the production environment~~ fix(assets): ensure ?no-inline is not included in the asset url in the production environment Apr 23, 2025

hi-ogawa merged commit 16a73c0 into vitejs:main Apr 23, 2025
18 checks passed

OnlyWick deleted the feat-no-inline-vite branch April 23, 2025 14:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(assets): ensure ?no-inline is not included in the asset url in the production environment #19496

fix(assets): ensure ?no-inline is not included in the asset url in the production environment #19496

OnlyWick commented Feb 23, 2025 •

edited

Loading

This comment was marked as outdated.

sapphi-red commented Feb 26, 2025

OnlyWick commented Feb 26, 2025

sapphi-red commented Feb 27, 2025

OnlyWick commented Feb 27, 2025 •

edited

Loading

Techn1x commented Apr 2, 2025

sapphi-red left a comment

Techn1x commented Apr 23, 2025 •

edited

Loading

hi-ogawa left a comment

hi-ogawa commented Apr 23, 2025

sapphi-red commented Apr 23, 2025 •

edited

Loading

OnlyWick commented Apr 23, 2025

Techn1x commented Apr 24, 2025

fix(assets): ensure ?no-inline is not included in the asset url in the production environment #19496

fix(assets): ensure ?no-inline is not included in the asset url in the production environment #19496

Conversation

OnlyWick commented Feb 23, 2025 • edited Loading

Description

This comment was marked as outdated.

sapphi-red commented Feb 26, 2025

OnlyWick commented Feb 26, 2025

sapphi-red commented Feb 27, 2025

OnlyWick commented Feb 27, 2025 • edited Loading

Techn1x commented Apr 2, 2025

sapphi-red left a comment

Choose a reason for hiding this comment

Techn1x commented Apr 23, 2025 • edited Loading

hi-ogawa left a comment

Choose a reason for hiding this comment

hi-ogawa commented Apr 23, 2025

sapphi-red commented Apr 23, 2025 • edited Loading

OnlyWick commented Apr 23, 2025

Techn1x commented Apr 24, 2025

OnlyWick commented Feb 23, 2025 •

edited

Loading

OnlyWick commented Feb 27, 2025 •

edited

Loading

Techn1x commented Apr 23, 2025 •

edited

Loading

sapphi-red commented Apr 23, 2025 •

edited

Loading