feat: support custom modifiers for glob imports

[brillout]

Description

Add support for custom modifiers for glob imports.

import.meta.globEager('/pages/**/*.page.client.js', {
  assert: { type: 'some-custom-modifier' }
})

Additional context

Two frameworks that are being built on top of vite-plugin-ssr need this.

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.
• [ x 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.

[bluwy]

Note: We might want to refrain extending assert with #7017. Supporting custom modifiers is an interesting idea though, and appending a ? suffix with it sounds like a fine approach, since the import reflection proposal isn't supported by bundlers yet.

[brillout]

@bluwy Thanks for mentioning #7017.

delay further changes in import modifiers

@patak-dev I understand the motivation to delay further import modifiers. The thing is that this PR is a blocker for @thetre97 who is building a Gridsome-like framework on top of vps, and it's also a blocker for the framework I'm currently building on top of vps and Telefunc.

Maybe, if it's ok from your perspective, we could merge this is in and keep it undocumented until we figure out #7017.

Supporting custom modifiers is an interesting idea though

This is not really something new, vps already uses a custom modifier ?meta. This PR just makes it work with glob imports.

[bluwy]

This is not really something new, vps already uses a custom modifier ?meta. This PR just makes it work with glob imports.

Yeah, I mean making assert.type synonymous as ?${assert.type}. I don't think assert.type: "raw" was meant to work that way.

Re merging first, I think we can still implement #7017 first though. Most of the ground work has be done to support the assert.type syntax, and it's not until a few more days before the beta is released (I think). We just need someone to do it 😄

[brillout]

Makes sense.

We talked about this PR and we think we should add { as: 'raw' } for glob import marked as experimental in Vite 2.9 and deprecate the { assert: { type: 'raw' }} syntax. We can then remove the assert syntax in Vite 3.0 at the beginning of May.

I'll update my PR to also implement this.

[patak-dev]

@brillout would you explain the complete use case for both frameworks? What modifiers do they need? Or do they need this to be open-ended for users?

[brillout]

Imagine a framework some-framework built on top of vps (vite-plugin-ssr), with following file:

// node_modules/some-framework/renderer/_default.page.client.js

export { render }

import ReactDOM from 'react-dom'
import React from 'react'

async function render(pageContext) {
  const { Page, pageProps } = pageContext
  ReactDOM.hydrate(
    <Page {...pageProps} />,
    document.getElementById('view')
  )
}

To take control over render(), the user can override _default.page.client.js:

// pages/_default.page.client.js (this file lives in user-land)

export { render }
// Override the `_default.page.client.js` provided by `some-framework`:
export const overrideDefaults = true

import ReactDOM from 'react-dom'
import React from 'react'

async function render(pageContext) {
  // Here the user takes control over how her pages are rendered/hydrated
}

Now the thing is that Node.js needs to know which _default.page.client.js should be added to the HTML. But Node.js cannot load _default.page.client.js files (they are are meant to be loaded only in the browser).

So vps uses a custom modifier ?meta to transform _default.page.client.js?meta into this:

// pages/_default.page.client.js?meta

export const exportNames = ['render', 'overrideDefaults']

That way Node.js can have some meta information about the _default.page.client.js file without having to load it.

Now, with the vps changes I'm currently working on, the client-side also needs meta information about *.page.client.js files. In other words, ths vps client needs this:

// node_modules/vite-plugin-ssr/client/index.js

// Each `.page.client.js` file is loaded only if needed
const pageFiles = import.meta.glob('/**/*.page.client.js')

// Load the meta information of all `.page.client.js` files
const pageFilesMeta = import.meta.globEager('/**/*.page.client.js', { as: 'meta' })

That way the vps client can have meta information about all .page.client.js files without having to load them all. If the vps client would need to load all .page.client.js files that would explode bundle size.

The ?meta custom modifier is similar to the test's customModifierPlugin() of this PR: https://github.com/brillout/vite/blob/0688eb408d6b409bfd8feeaad5d0e046adfc95f0/packages/playground/glob-import/vite.config.ts#L14-L33.

Let me know if something's not clear or if you have any question.

[brillout]

I'll update my PR to also implement this.

Done. This PR now also implements #7017.

[patak-dev]

Would it be possible to split this PR in two so we can merge the fix for #7017 during the 2.9 beta? The feat part still needs to be discussed in a team meeting (probably this Friday). If not, I'll send a PR based on your work to implement the migration to as

[brillout]

Will do.

Re the feat part. The way I see it is that it's a bug fix more than a feature (the reason I marked it as feat: is because I know from past experience that we have a slightly different view on what is a bug fix VS what is a feature :-)). Point being: if the user can already import './someFile.js?meta' today, why should we forbid the user to import.meta.glob('./someFile.js', { as: 'meta' })? Either we forbid custom modifiers everywhere or we allow custom modifiers also in glob imports. From a consistency point of view, I'd say this PR is a bug fix.

But maybe I'm missing something here :-).

[patak-dev]

Fair enough, but we would still need to discuss this with the team. I personally think that you have a point and reading through https://github.com/tc39/proposal-import-reflection, it looks like the <reflection-type> is open-ended and not defined by the standard. At least not by this proposal, that only names one:

import FooModule from "./foo.wasm" as "wasm-module"

We know that user suffixes will only collide with Vite and the ecosystem, but with the as we may collide with these keyboards. So we may need some kind of guard like { as: 'vite:meta' }

[brillout]

👍 Makes sense. Personally, I've a slight preference for the nicer aesthetics of omiting a guard over the minor reliability increase of having a guard. But I'm fine either way :-).

PR is now split:

• Fix: fix: deprecate { assert: { type: raw }} in favor of { as: raw } (fix #7017) #7215.
• Feat: this PR will contain only the feat commit after the fix is merged.

[brillout]

This PR now contains only the feat. It's rebased and ready to be merged.

Sum up of why vps needs this.

vps has a ?meta transformer:

// pages/product.page.client.js

export const useClientRouter = true

// Some Vue/React component
export const Page = /* ... */

// This file can be quite large
/* ... */

// pages/product.page.client.js?meta (after transformation)

export const exportNames = ['useClientRouter', 'Page']

// This file is now only one LOC

(The meta transformer uses es-module-lexer to extract the ESM export names, similar to the example of this PR.)

Thanks to the meta transformer, the vps client router can do this:

// node_modules/vite-plugin-ssr/dist/client/router.js

// The vps client router needs to know all `export` of all `.page.client.js`

const clientPageFilesMeta = import.meta.globEager('/**/*.page.client.js', { as: 'meta' })
console.log(clientPageFilesMeta['/pages/product.page.client.js'].exportNames) // ['useClientRouter', 'Page']

(It would be prohibitive to import.meta.globEager('/**/*.page.client.js') as it would mean that the browser would load all pages at once.)

I'm currenlty working on a vps refactor and the refactor needs this. (And the frameworks being built on top of vps need this refactor.)

[patak-dev]

@brillout we discussed with part of the team in the last meeting, and what was proposed was that this should be supported by a plugin at the moment. Maybe as import.meta.globEagerMeta, or using the syntax you are proposing and transforming it yourself to expand the imports (ping @antfu, @bluwy). One of the issues with continuing to expand on the current syntax is that we don't know how these proposals will evolve. We'll leave this PR for re-discussion in a future meeting though.

[brillout]

vps now uses a custom plugin instead of a glob import.

Other than HMR, it works, and I've an idea for making HMR work.

Although vps doesn't need this PR anymore, I still believe we should merge it. Let me elaborate.

The reason we don't merge this is because we are not sure whether we will keep supporting the { as: ... } syntax.

Let's imagine we deprecate this syntax and what merging this PR would then mean.

If we don't merge this PR, the message will be:

If you use import.meta.glob('someGlob', { as: 'raw' }), please change it to...

If we do merge this PR, the message will be:

If you use import.meta.glob('someGlob', { as: 'someModifier' }), please change it to...

The bottom line: the two messages are not really different.

the issues with continuing to expand on the current syntax

Precisely, we do not extend the syntax. It's only one syntax. Either we continue to support the syntax or we depreate it, but either way it doesn't really make a difference what the user does with that syntax.

That said, merging this PR does mean that more users will have to move to the new syntax. But I don't think we should block users from using custom modifiers because we are not sure about the syntax. It seemed to me that we already settled for the { as: ... } syntax, so I don't see why we don't stick to that decision.

One thing we can do is to mark the { as: ... } syntax as Experimental or Beta. (Like we already do for the Vite's SSR native API.)

If we want to be very conservative, we can even warn the user with something like:

This feature is meant only for Vite frameworks, do not use this if you are a end-user. Any minor may introduce breaking changes to the { as: ... } syntax.

This essentially enables us to deprecate the syntax whenever we want.

Either way, supporting the { as: ... } syntax only half-way doesn't make sense to me.

Writing the plugin did consume quite some time and we should spare the Vite ecosystem that effort.

Custom modifiers are crucial.

Beyond ?extractExportNames (I renamed ?meta), vps now uses a second custom modifier ?extractStyles.

vps now has first-class support for using Vue/React/... only on the server-side (zero/minimal browser-side JavaScript) and the ?extractStyles plays a crucial role: it removes all JS so that the client bundle can include all CSS imported by the server-side code, without including any of the server-side JS.

Such modifier will also be needed for React Server Components. (It's a new React 18 technique to write server-side components that never get loaded in the browser — in order to reduce browser-side bundle size.) (@frandiox I don't know if/how Hydrogen goes around that problem but let me know if you are interested in such modifier; I can share/publish it.)

Note that both ?extractExportNames and ?extractStyles apply to /**/*.page.server.js user files, which means that they need to be used in globs (or the the plugin I wrote).

There are probably other use cases for custom modifiers we are currenlty not forseeing.

Looking forward to ship vite-plugin-ssr@0.4.0. (Regardless of whether we merge this PR :-).)

[antfu]

The reason I propose to start with a plugin on userland is that

• This is rather a complex thing to fit all needs
• Iterating in core takes a long time (a.k.a very long feedback loop)
• And it could take a lot of discussions since we don't want to break existing usage, or at least before v3.0
• Using a plugin would give you the full freedom of experimenting with different ideas and iterating faster. Breaking changes is way either to introduce in a plugin than the core.

And then if the community come up with a good interface that fits most of the needs, we could port it back to the core and released it as a part of v3.0 to only introduce breaking changes one.

[brillout]

This is rather a complex thing to fit all needs

I'm guessing you mean custom modifiers in general, since this PR in itself is trivial.

The trick some/file.${fileExtension}?someModifier&lang.${fileExtension} resolves all potential problems I forsee. It actually already is a reliable solution for the Vue plugin. Also note that vps has quite an extensive test suite with integrations with all kinds of libraries. If vps tests are green that's a significant signal. But really the strongest signal here is that the Vue plugin already uses that trick.

Iterating in core takes a long time (a.k.a very long feedback loop)

This PR only adds 3 LOCs to core: https://github.com/vitejs/vite/pull/7209/files#diff-b43945059cc51ad52c7854aafa9e0435ecd8312e0740715d99fb6c6f5545cae1R154-R156.

And it could take a lot of discussions since we don't want to break existing usage, or at least before v3.0

This PR does not break anything. As for breaking the { as: ... } syntax, we can communicate this syntax as an Experimental/Beta feature. We actually may want to do this regardless of whether we merge this PR, since v2.9 will support { as: 'raw' } anyways.

Using a plugin would give you the full freedom of experimenting with different ideas and iterating faster. Breaking changes is way either to introduce in a plugin than the core.

In principle I agree but not for this specific case. The only thing my plugin does is to circumvent the fact that this PR is not merged. There is nothing the plugin enables me to experiment.

It's actually the opposite: merging this PR fosters the Vite ecosystem to experiment with custom modifiers. Since this PR further enables custom modifiers.

If we want to be highly conservative, we can even add a flag vite.config.js#experiments.enableCustomModifierGlobs.

Bottom line:

• We seem to be hesitant about the { as: ... } syntax, so we probably should add a Beta/Experiment warning for it. (Regardless of this PR since we already support { as: 'raw' }.)
• We should merge this PR, fostering the Vite ecosystem to implement and experiment custom modifiers.

[frandiox]

Such modifier will also be needed for React Server Components. (It's a new React 18 technique to write server-side components that never get loaded in the browser — in order to reduce browser-side bundle size.) (@frandiox I don't know if/how Hydrogen goes around that problem but let me know if you are interested in such modifier; I can share/publish it.)

I indeed tried to use glob modifiers a few months ago when implementing RSC the first time and was surprised that it wasn't supported (I believe this came up in a conversation with Evan on Hydrogen and RSC). We ended up finding a different approach but I think this feature could still help.

I don't have a strong opinion on syntax or whether this should be in core or in a separate plugin first to test it out. That said, it is something that feels like it should be supported out of the box just like normal imports 🤔

if you are interested in such modifier; I can share/publish it.)

I definitely am 👍

[brillout]

I definitely am 👍

https://github.com/brillout/vite-plugin-ssr/blob/70d85039b806b1ee09ad1eca3d9e71e0968fe180/vite-plugin-ssr/node/plugin/extractStylesPlugin.ts — I added thorough comments but let me know if something's not clear. In case you play with it, make sure to use DEBUG=extractStyes.

The plugin that re-implements the import.meta.glob functionality: https://github.com/brillout/vite-plugin-ssr/blob/70d85039b806b1ee09ad1eca3d9e71e0968fe180/vite-plugin-ssr/node/plugin/virtualPageFilesExportNames.ts. But it turns out HMR is tricker to make work than I initially thought. Even worse is that the root problem is cache invalidation. This means that new/removed glob matches will be ignored leading to quite some confusing bugs (it corrupts vps in non-obvious ways and makes a vps app seem like it randomly fails). @patak-dev is there anything I can do to fasten up the process of reviewing this PR?

[brillout]

I'm closing this PR in favor of vite-plugin-glob. I'm confident vite-plugin-glob will become a Vite built-in plugin. We can re-open this PR if it doesn't.

I propose to start with a plugin on userland

@antfu That was a great idea :-).