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

feat: experimental.buildAdvancedBaseOptions #8450

Merged
merged 17 commits into from Jun 20, 2022
Merged

feat: experimental.buildAdvancedBaseOptions #8450

merged 17 commits into from Jun 20, 2022

Conversation

patak-dev
Copy link
Member

@patak-dev patak-dev commented Jun 2, 2022

Description

Fix #3522

Building on top of:

Read the description in #7644 first for context. See also @danielroe's comment: #3522 (comment) where he describes why frameworks like Nuxt need dynamic/runtime base support. And why they would like to be able to define a separate base for hashed assets and for public files (the caching strategies could be different, and they may be in different CDNs).

In discussions with @Rich-Harris, they also expressed interest in being able to set relative base for in-assets paths and an absolute base for in-html paths. @matthewp and @FredKSchott also showed concerns about relative base when dealing with 404s. This PR may also help in that regard, but I need to better understand the use case.

It should be working already except for the plugin legacy and the manifest generation. Pushing it so we can start discussing the API while we take care of the implementation details.

There is a new config experimental.buildAdvancedBaseOptions: BuildBasedConfig, defined as:

export interface BuildAdvancedBaseOptions {
  relative?: boolean
  url?: string
  runtime?: (filename: string) => string
}

export type BuildAdvancedBaseConfig = BuildAdvancedBaseOptions & {
  assets?: BuildAdvancedBaseOptions
  public?: BuildAdvancedBaseOptions
}

If defined, these functions will be called for assets and public files paths that are generated in JS assets. For assets and public files paths in CSS or HTML, the corresponding url base urls or global base will be used. Example of a config with a dynamic function:

export default {
  experimental: {
    buildAdvancedBaseOptions: {
      url: 'https:/cdn.domain.com/'
      runtime: (url: string) => `window.__toAssetPath(${url})` 
    }
  }
}

Nuxt (through @danielroe) requested being able to use different bases for hashed assets and public files, as both may need different cache strategies. The PR implements this by letting you override the global build base options using buildBaseOptions.assets and buildBaseOptions.build. When using relative base, the runtime function isn't needed for assets as all the asset paths will be computed using import.meta.url. The public.runtime function is still useful if the public files aren't deployed in the same base as the hashed assets

Example exercising all the features:

  • separate assets and public files base configuration
  • dynamic base in assets so the server can inject the correct URL without rebuilding the assets
  • absolute URL for in-HTML assets and public files paths (this could be a placeholder that is later replaced by the server avoiding the need to rebuild the HTML). Some frameworks like SvelteKit have their own logic and they will need to replicate the handling this PR is doing if to respect these config options.
export default {
  experimental: {
    buildAdvancedBaseOptions: {
      assets: {
        url: 'https:/cdn.domain.com/assets',
        runtime: (url: string) => `window.__assetsPath(${url})` 
      },
      public: {
        url: 'https:/www.domain.com/',
        runtime: (url: string) => `window.__publicPath + ${url}` 
      }
    }
  }
}

There is a replicated assets playground test using these new options. You can run it using

cd playground/assets
pnpm run build:runtime-base
pnpm run preview:runtime-base

For preview to work, it is expected that the user will build locally using a configuration that ends up generating assets and public files that can be served by a single server pointing at base. This is the same as when currently using an external URL as base. Frameworks could provide middlewares for redirecting these URLs to the local assets. We could maybe support this by default, I need to check the details.

We could deprecate the base relative shortcut ('./' or ''). For the moment the config is properly resolved to build.baseOptions.relative: true.

Alternatives

I considered having base: string | AdvancedBaseConfig instead of base + build.advancedBaseOptions, but I think it is better to have these options under build. When we use relative base, we are already setting the base as '/' for the dev and preview server. So relative base only affects build. The same goes for a runtime base, which uses the same mechanism as relative base to inject a JS function in the middle of an in-JS CSS string. And when we have an external URL as a base, it also only applies during build time.
So I think it is better to leave the global base as a global default + dev/preview setting and push all these build-only advanced options inside build.

A new hook to resolve these assets could be considered, or make the advanced build option a function. The problem with these is that context is needed to be passed (importer chunk name and type). And we need to also document that the user should return "\"+...+\"" string. This exposes the internal handling of strings and will limit us later if we want to move to template strings for example. We can't perform optimizations based on the options passed, if only a static URL is needed then we could avoid some indirect generation. So IMO, explicit options are better here.

edit: move from build.advancedBaseOptions to experimental.buildAdvancedBaseOptions


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@patak-dev patak-dev added the p3-downstream-blocker 🔨 Blocking the downstream ecosystem to work properly (priority) label Jun 2, 2022
@patak-dev patak-dev requested a review from antfu June 2, 2022 23:03
@patak-dev
Copy link
Member Author

We discussed with @Shinigami92 @sodatea and @sapphi-red, and the consensus was that we should try to hide these advanced options from the regular user (they are intended to help Framework authors more than final users, and are probably going to be used in the context of deploy adaptors?). So the suggestions were:

  • Don't remove the relative base shortcut, as it is something that most users will use (to deploy to github pages for example)
  • Rename build.baseOptions to base.advancedBaseOptions to make the intent more clear
  • Rename dynamic to runtime (unrelated to the above, but sounds more clear)

@patak-dev patak-dev changed the title feat: build.baseOptions (dynamic base support) feat: build.advancedBaseOptions (+runtime base support) Jun 3, 2022
@sapphi-red
Copy link
Member

Don't remove the relative base shortcut

I remembered this one now. (A bit off-topic but leaving a note here)

We'll need to discuss with the team if we want to deprecate both '' and './' in favor of 'auto', or if we decide for '' or './' and deprecate with a warning or an error in v3 the other one.
#8387 (comment)

@davidwallacejackson
Copy link
Contributor

I don't have time to test this PR because there's a lot of code I'll have to refactor to get away from our current strategy for handling multiple hosts -- but I believe that the changes here would solve my team's problems long-term and I think they belong in core.

For a little context on our own use case, we build our application for four different targets:

  • production
  • qa
  • beta
  • onprem (i.e. to be included in a docker container and hosted on clients' own infra)

In the first three targets, the application lives on one domain (wandb.ai) while all the assets live on a CDN (cdn.wandb.ai). We've made this work by manually injecting a placeholder URL, which we then replace in CI, once for each target.

The onprem build is more complicated, though, since we have no knowledge ahead of time of exactly how it'll be hosted. To account for that, we ship the frontend build with the placeholder still in place, and perform replacement at runtime once we know where it'll be served. At least some of this replacement could be obviated by these changes, though not all. I also want to call out that for teams with many targets like this the alternative is to build n copies of their frontend (this is what we used to do) which, even with a super-performant build system, is extremely inefficient! I think there's a lot of value in Vite having a good story about how to support this.

@danielroe
Copy link
Contributor

This is really good to see; the API looks like it allows for a full range of customisation. Many thanks. 👌

@patak-dev patak-dev changed the title feat: build.advancedBaseOptions (+runtime base support) feat: experimental.buildAdvancedBaseOptions (+runtime base support) Jun 20, 2022
@patak-dev
Copy link
Member Author

We discussed this PR with the team, and we can't commit to an API until we get more feedback from the ecosystem. We decided to release the API in its current form as an experimental feature in v3. If you are interested in seeing these PR features as stable, please get involved. You can directly comment here or join https://chat.vitejs.dev to chat.

@patak-dev patak-dev changed the title feat: experimental.buildAdvancedBaseOptions (+runtime base support) feat: experimental.buildAdvancedBaseOptions Jun 20, 2022
@patak-dev patak-dev merged commit 8ef7333 into main Jun 20, 2022
15 of 18 checks passed
@patak-dev patak-dev deleted the feat/base-config branch June 20, 2022 15:27
@benjaminprojas
Copy link

benjaminprojas commented Jun 23, 2022

I just came across this PR when it was mentioned to me here: jy0529/vite-plugin-dynamic-publicpath#13 (comment)

I'd love to share how this API benefits us and our use-case.

My team and I currently develop AIOSEO, the original SEO plugin for WordPress. Our plugin is currently running on over 3 million websites. This product has been on the market for 15 years as of today. About 2 years ago we did a complete rebuild of the plugin using Vue/Vue CLI.

Since all of our code lives inside our users websites, we have no way of knowing their domain name or path to the installed files until runtime. When we used Vue CLI we were able to use the handy webpack variable __webpack_public_path__ to fix this "on the fly".

However when we switched our build process to Vite a few months ago (🎉) there was no way to do this directly. We've been relying on the vite-plugin-dynamic-publicpath package to address this for us and this works okay, with some limitations. For example, we can't use import.meta.url anywhere as it always points to the wrong path. Another limitation is with web workers; we can only use web workers by adding them inline: ?worker&inline

We then have to rewrite the path to the URL's when in production like so:

if (import.meta.env.PROD) {
	const publicPath = window.aioseo.urls.publicPath || '/'
	window.__aioseo_dynamic_handler__ = importer => {
		return `${publicPath}dist/${import.meta.env.VITE_VERSION}/assets/${importer.replace('./', '')}`
	}
	window.__aioseo_dynamic_preload__ = preloads => {
		return preloads.map(preload => {
			return `${publicPath}dist/${import.meta.env.VITE_VERSION}/assets/${preload}`
		})
	}
}

Having the ability to do all of this directly inside vite would be 💯.

Our team is 100% behind vitejs/vue (even in the WP block editor where everyone else uses react!). We would really love to see this as a core feature.

@patak-dev
Copy link
Member Author

Thanks for taking the time to share this use case @benjaminprojas, it looks like a perfect fit for this feature. And the current API should already allow you to do exactly what you want (docs for vite@3.0.0-beta.1 here https://main.vitejs.dev/guide/build.html#advanced-base-options). We are still discussing the final API (we have some ideas to make it more flexible), and it may completely change before v3, so please pin your Vite version if you use it. We are probably going to release it as experimental in v3, and try to keep it stable, as we will have another chance to rethink the API once we move from experimental to a stable feature.
If you test the API in your projects, please come back with feedback about how it is working for you.

@sciyoshi
Copy link

Not sure if this is a bug or not, but the experimental.renderBuiltUrl option does not seem to apply to the asset preloading code. We are planning on using this option to load assets from a different CDN depending on which environment the app is deployed to, and we'd rather not run different builds for each environment.

@sapphi-red
Copy link
Member

@sciyoshi There's a issue here #9680.

@paulcgdev
Copy link

This doesn't seem to support dynamic imports as in:

import React, { lazy, Suspense } from 'react'

const TodosList = lazy(() => import(
  './components/TodosList'
  )
)

const App = () => {
    //...
    <Suspense fallback={ <div>Loading...</div> }>
        <TodosList todos={ todos } />
    </Suspense>
}

The app directory is nested from site root hence the resulting TodosList.{hash}.js chunk ends up with incorrect URL and throws 404. Is there any workaround for this or am I doing/missing something?

@patak-dev
Copy link
Member Author

@sciyoshi @paulcgdev would you review if #9938 would accommodate your use cases:

@paulcgdev
Copy link

@patak-dev it looks likely it might work for my use case since I just need to do the same thing I'm doing for the asset files on per app dir for the dynamic chunks in the renderBuiltUrl (or wherever this ends up when out of experimental). Currently, I have something like:

experimental: {
  renderBuiltUrl: (fileName : string, { hostType }) => {

    if (hostType === 'js') {
      return {
        runtime: `window.myAppDir.url + '/dist/${ fileName }'`
      }
    }

    return fileName
  }
}

...which correctly loads the asset files on build for example from src/apps/page/settings/assets/react.svg.

The backend is from a WordPress plugin hence the root dir would be nested in something like https://domain.tld/wp-content/plugins/my-plugin. So the asset file in the example above would be loaded with https://domain.tld/wp-content/plugins/my-plugin/dist/apps/page/settings/assets/react.{hash}.svg and so, I just need the same thing for dynamic imports.

@patak-dev
Copy link
Member Author

@paulcgdev yes, it should work after #9938 using only experimental.renderBuiltUrl. The only issue is that window.myAppDir.url + will be appended to each so there may be a bit of repetition, and this replacement is done after minification so maybe a helper could be used in case that is an issue.

@paulcgdev
Copy link

@patak-dev should be better than nothing... so, looking forward to trying it out. 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p3-downstream-blocker 🔨 Blocking the downstream ecosystem to work properly (priority)
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

dynamic publicPath support
7 participants