feat!: relative base by patak-cat · Pull Request #7644 · vitejs/vite

patak-cat · 2022-04-07T23:07:36Z

fix #2009
fix #5270

Description

Makes built apps portable when using relative base (base: './' or base: '').

When using a relative base, there shouldn't be any absolute URL. Including paths to public files and paths to assets from HTML files.

Related discussions:

Why we need relative base support, from @Rich-Harris's comment:

the basepath is configured by an environment variable at runtime

the site is deployed with IPFS

you're building a Chrome extension

the site is being viewed on something like the Internet Archive

Update to modern browser targets

Edit: We discussed this in a team meeting and we have agreed to update the browser targets as described here in v3

We need support for import.meta.url to be able to find the assets URL in some cases (for the preload URLs for example).

import.meta is at 91.22%, and we are currently require dynamic import support so we are at 91.47%. Given that import.meta.url will enable us to implement features like this PR, and the numbers will keep getting better, I think we could change our default modern target in Vite v3 and push an extra 0.25% of users to legacy mode. See Discussion with @bluwy and @sapphi-red in Discord here

The new esbuild resolved target upgrades 'es2019' to 'es2020' and downgrades safari from 13.1 to 13 so we keep transpiling nullish coallesing (still at ~89%)

resolved.target = [
  'es2020', // support import.meta.url
  'edge88',
  'firefox78',
  'chrome87',
  'safari13' // transpile nullish coallesing
]

@vitejs/plugin-legacy is updated to check import.meta support (on top of dynamic import support):

const detectModernBrowserCode = `try{import(new URL(import.meta.url).href).catch(()=>1);}catch(e){}window.${detectModernBrowserVarName}=true;`

Asset URLs replacements in JS files

Edit: after a discussion with @bluwy, he proposed we keep using quotes and JSON.stringify and interpolate using " + ... + ".

Alternative interpolation strategy that is no longer used in the PR

For assets paths imported from a JS file, or for CSS inlined with asset URLs, we need to use import.meta.url to obtain a runtime absolute path because these strings can be re-exported and used in other files, or used directly in the DOM.

Instead of plain strings

return `export default ${JSON.stringify(css)}`

We could use template literals in these cases

// Return a template string so we can use interpolation when replacing __VITE_ASSET__
return `export default \`${css}\``

Allowing us to inject runtime interpolation with new URL(...,import.meta.url)

const replacement = isRelativeBase(config.base)
  ? `\${new URL(${JSON.stringify(outputFilepath)},import.meta.url)}`
  : outputFilepath
s.overwrite(match.index, match.index + full.length, replacement, {
  contentOnly: true
})

Note

During the Vite 2.0.0 beta, 00bc446 added relative base support, fixing #1669. Afterward, tests for relative base weren't included in Vite's CI. So the code evolved and we lost support for this feature.

What is the purpose of this pull request?

Bug fix
New Feature
Documentation update
Other

patak-cat · 2022-04-07T23:19:31Z

Tests are passing, except for Windows. We are probably missing a path normalization.

More testing is still needed, I changed some playgrounds to use relative base, and tested by hand in some others (there is a new build:portable script in the assets playground for example). It isn't easy to test a playground with both bases without doubling the time and making the expectation more complex. So maybe some playgrounds using relative base, a new portable playground for some specific checks, and adding the build:portable, preview:portable to most playgrounds for manual debugging could be enough.

sapphi-red · 2022-04-08T09:15:58Z

Looks like tests are failing because this line is missing normalizePath. (I have not checked all the code but at least it worked for playground/vue and playground/worker)

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

Line 162 in c02537b

const file = path.resolve(path.dirname(id), rawUrl.slice(1, -1))

In addition, I think this PR will fix #5270 and #7279 since import.meta won't be transpiled.

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

patak-cat · 2022-04-08T11:54:05Z

I don't know how disruptive it could be for the ecosystem, but relative paths seem like a better default to me than base: '/'. I think users could even open a built app from the filesystem without needing a local preview server 🤔
Should we analyze changing the default base in Vite 3.0? For sure we would need a major, we would need to wait if not another year.

Note: Right now, during dev even if the user sets base: './', Vite will cheat and use '/' that makes some handling easier in the dev server. I think this is fine, I'm thinking if we could still try to use relative base during dev as much as possible so Dev and Build are closer but I don't know if the extra complexity is justified.

haoqunjiang · 2022-04-08T14:34:54Z

Posting my reply in the Discord channel here for a more public discussion:

The relative base path doesn't work well for code-splitting. That's why webpack introduced publicPath: 'auto' even though ./ already works. webpack/webpack#11127 (./ is relative to the page URL, auto is relative to the script URL, i.e. import.meta.url, which may be a CDN URL)

Note that even if we follow the webpack approach, the auto public path has some edge cases in legacy browsers that cannot be fully resolved:
Webpack recommends falling back to currentScript in legacy bundles, but it's not equivalent to import.meta.url in functionality. It will fail in callbacks or event handlers.
https://developer.mozilla.org/en-US/docs/Web/API/Document/currentScript

It's important to note that this will not reference the <script> element if the code in the script is being called as a callback or event handler; it will only reference the element while it's initially being processed.

patak-cat · 2022-04-08T14:53:43Z

I think what this PR is doing for './' is similar to Webpack's publicPath: 'auto', as it is using import.meta.url as you describe.

I like having a base: 'auto', it is more clear than './' or '' because that isn't the base used all the time. For simplicity, we could make all of them work in the same way, since './' is the same as 'auto' in the cases where it works. Maybe we could add a warning when the user uses './' or '' deprecating them and pointing to the new 'auto' instead.

packages/vite/src/node/build.ts

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

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

Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>

patak-cat · 2022-04-14T18:43:19Z

@benmccann would you be able to review this PR and check if most use cases discussed in sveltejs/kit#595 (or involve others from the Svelte community that may be interested in helping push this change forward?)

@wighawag it would be good if you could check this PR, and see if it would help you with https://github.com/wighawag/sveltejs-adapter-ipfs

For context, this PR may be later complemented with an option to define the URL for the assets path dynamically (and public files path, but I think SvelteKit isn't using the public folder), #3522 (comment) (still keeping relative as many of the URLs as possible)

packages/vite/src/node/utils.ts

benmccann · 2022-04-18T14:20:03Z

@benmccann would you be able to review this PR and check if most use cases discussed in sveltejs/kit#595 (or involve others from the Svelte community that may be interested in helping push this change forward?)

@Rich-Harris might be the best person to take a look at this one. I see @dominikg and @bluwy have both taken a look at this PR as well though

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

packages/vite/src/node/utils.ts

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

Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>

Rich-Harris · 2022-05-17T19:42:32Z

Confirmed that this now works for a SvelteKit test case, the only change needed being to change base in our internal config to ./. Wonderful work, thank you so much @patak-dev! Can't wait to use this 😍

patak-cat · 2022-05-18T05:07:10Z

We already discussed with the team and approved changing the modern browser targets increasing the baseline to browsers supporting import.meta.url (that would be useful moving forward not only for this PR). The implementation got a few reviews in the past few days (see reviews invalidated by added test cases and minor fixes in the last commits). And we now have success stories from SvelteKit and partial testing also from Nuxt (@danielroe was also testing it yesterday).

Let's merge the PR so we can start building on top of it. There are changes I think would be good to make but it will be easier to discuss them in separate PRs. It is also better if we can later release a new v3 alpha patch with it so we start getting more testing from the ecosystem.
Thanks to the folks that tested along the way, please create issues from now on so we can stabilize things for v3. @wighawag I'm especially interested in your use case as it will be a big stress test for these changes.

patak-cat · 2022-05-18T10:49:50Z

Released as part of vite@3.0.0-alpha.1

evilgeniuscreative · 2023-06-09T18:41:46Z

I still find this very difficult to actually make happen, and the documentation is not as clear on how to do it and specifically what it does as I think Vite thinks it is.

patak-cat added 2 commits April 8, 2022 00:16

feat: relative base

cb42862

fix: don't use template string in dev

bdacc9f

patak-cat requested a review from bluwy April 7, 2022 23:12

patak-cat added the p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority) label Apr 7, 2022

patak-cat added this to the 3.0 milestone Apr 7, 2022

fix: windows

7e4484a

sapphi-red reviewed Apr 8, 2022

View reviewed changes

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

chore: stringifyAsTemplateLiteral util

7744ca3

bluwy reviewed Apr 13, 2022

View reviewed changes

packages/vite/src/node/build.ts Outdated Show resolved Hide resolved

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

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

patak-cat mentioned this pull request Apr 13, 2022

fix(worker): import.meta.* #7706

Merged

9 tasks

patak-cat and others added 2 commits April 13, 2022 21:30

chore: typo

3bd450d

Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>

chore: merge main

2db74e8

patak-cat mentioned this pull request Apr 14, 2022

Set base with environment variable #7743

Closed

4 tasks

dominikg reviewed Apr 14, 2022

View reviewed changes

packages/vite/src/node/utils.ts Outdated Show resolved Hide resolved

patak-cat added 2 commits April 14, 2022 22:06

chore: remove unused regex

1491bb9

fix: resolve worker TODO

77a05e3

bluwy reviewed Apr 18, 2022

View reviewed changes

iacore mentioned this pull request Apr 23, 2022

Way to get base relative path sveltejs/kit#3376

Closed

Rich-Harris mentioned this pull request Apr 23, 2022

font use absolute path assuming the font file will be hosted on the route. sveltejs/kit#1477

Closed

iacore mentioned this pull request Apr 25, 2022

fix: relative base code generation (previously generated as ./) #7905

Closed

patak-cat mentioned this pull request Apr 27, 2022

Load all files from the entry location #7930

Closed

4 tasks

patak-cat and others added 2 commits April 29, 2022 20:37

fix: apply fix by @locriacyber

69d872f

chore: typo

41d9884

Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>

benmccann previously approved these changes May 17, 2022

View reviewed changes

test: worker with relative base

f785bed

patak-cat dismissed stale reviews from benmccann and antfu via f785bed May 17, 2022 21:26

patak-cat added 2 commits May 17, 2022 23:30

chore: merge main

b5b0a84

test: skip invalid serve tests for relative base

7f7d763

patak-cat mentioned this pull request May 17, 2022

Avoid adding leading/trailing slash to base #8202

Closed

4 tasks

fix: urls in windows

277f967

patak-cat changed the title ~~feat: relative base~~ feat!: relative base May 18, 2022

patak-cat merged commit 09648c2 into main May 18, 2022

patak-cat deleted the feat/relative-base branch May 18, 2022 05:07

patak-cat added the breaking change label May 18, 2022

andylizi mentioned this pull request May 18, 2022

Relative links not handled properly in nested html files #4517

Closed

7 tasks

This was referenced May 18, 2022

docs: update for default target change #8223

Merged

When Built with configure assetFileNames, the assets not found #4503

Closed

fix(plugin-legacy): prevent failed to load module #8285

Merged

tony-sull mentioned this pull request May 23, 2022

Allow relative base (linking of assets and base) / vite embeded deployment withastro/astro#3360

Closed

1 task

stefnotch mentioned this pull request May 23, 2022

Investigate relative base path stefnotch/vite-pseudo-template#2

Open

sapphi-red mentioned this pull request May 29, 2022

URL of public assets is wrong in built CSS files if assetsDir is set and base is './' #3650

Closed

6 tasks

patak-cat mentioned this pull request Jun 2, 2022

feat: experimental.buildAdvancedBaseOptions #8450

Merged

4 tasks

Yiniau mentioned this pull request Jun 13, 2022

base option not support fully url at dev mode #8527

Closed

7 tasks

sapphi-red mentioned this pull request Jun 14, 2022

fix: preload paths with different production base path breaks #3061

Closed

9 tasks

ScottPolhemus mentioned this pull request Jun 15, 2022

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

Merged

patak-cat mentioned this pull request Jul 13, 2022

feat!: support safari14 by default for wider ES2020 compatibility #9063

Merged

9 tasks

nvh95 mentioned this pull request Jul 20, 2022

fix: __VITE_PUBLIC_ASSET__hash__ in HTML #9247

Merged

12 tasks

zkrzyzanowski mentioned this pull request Jul 23, 2023

Interactive components not working since Astro updated to 2.0 angusjf/astro-integration-elm#1

Closed

Uh oh!

Conversation

patak-cat commented Apr 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Update to modern browser targets

Asset URLs replacements in JS files

Alternative interpolation strategy that is no longer used in the PR

Note

What is the purpose of this pull request?

Uh oh!

patak-cat commented Apr 7, 2022

Uh oh!

sapphi-red commented Apr 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

patak-cat commented Apr 8, 2022

Uh oh!

haoqunjiang commented Apr 8, 2022

Uh oh!

patak-cat commented Apr 8, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

patak-cat commented Apr 14, 2022

Uh oh!

Uh oh!

benmccann commented Apr 18, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Rich-Harris commented May 17, 2022

Uh oh!

patak-cat commented May 18, 2022

Uh oh!

patak-cat commented May 18, 2022

Uh oh!

evilgeniuscreative commented Jun 9, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

16 participants

patak-cat commented Apr 7, 2022 •

edited

Loading

sapphi-red commented Apr 8, 2022 •

edited

Loading