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: add base option for import.meta.glob #18510

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions docs/guide/features.md
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,26 @@ const modules = import.meta.glob('./dir/*.js', {
})
```

#### Base Path

You can also use the `base` option to provide base path for the imports:

```ts twoslash
import 'vite/client'
// ---cut---
const moduleBase = import.meta.glob('**/*.js', {
base: './base',
})
```

```ts
// code produced by vite:
const moduleBase = {
'dir/foo.js': () => import('./dir/foo.js'),
bluwy marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the key also start with ./ similar to without base?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right; it should definitely be consistent. I've made the changes.
7f0f5e7

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expect the key to be without ./ as '**/*.js' does not have ./ at the head. I think this depends on how to describe the base option. If we describe this as a prefix string, then I think we should remove ./ from the key. If we describe this as a base path, then I think we should add ./ to the key. A prefix string is more powerful than base path because it allows non-path prefix and maybe it works well with aliases, but the base path feels more natural with the name base.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't notice this PR is indirectly supporting '**/*.js' when base is set. I was thinking more like the latter, a base path to resolve the glob from and it feels easier to visualize (for me), e.g. as if you're resolving the glob in a different file path.

I think both behaviours should be equally powerful and can do the same things. But if the behaviour is more like a prefix, then perhaps the option should be called prefix, but then someone could ask for suffix too.

I also think the behaviour could be easier implemented if treated as a base path, so instead of using the file dir or root (for virtual modules), we could use whatever passed by base instead.

Copy link
Member

@sapphi-red sapphi-red Nov 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think both behaviours should be equally powerful and can do the same things.

I think the prefix one is a bit more powerful. For the following files,

  • foo-bar/a.js
  • foo-baz/b.js

if the base behaves as prefix, you can write import.meta.glob('**/*.js', { base: 'foo-' })['bar/a.js']. But if the base behaves as base, you cannot write that and need to write import.meta.glob('./**/*.js', { base: './' })['foo-bar/a.js'].

That said, I'm fine with going with base-like behavior.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I was thinking in base-like and will keep the change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also think we should keep base as a path and not as a prefix.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have modified the examples in the documentation to treat them as base instead of prefix.
500cb14

'dir/bar.js': () => import('./dir/bar.js'),
}
```

### Glob Import Caveats

Note that:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ export const cleverCwd2 = /* #__PURE__ */ Object.assign({"./modules/a.ts": () =>



});
export const customBase = /* #__PURE__ */ Object.assign({"modules/a.ts": () => import("./modules/a.ts"),"modules/b.ts": () => import("./modules/b.ts"),"modules/index.ts": () => import("./modules/index.ts"),"sibling.ts": () => import("./sibling.ts")});
export const customRootBase = /* #__PURE__ */ Object.assign({"a.ts": () => import("../fixture-b/a.ts"),"b.ts": () => import("../fixture-b/b.ts"),"index.ts": () => import("../fixture-b/index.ts")

});
"
`;
Expand Down Expand Up @@ -106,6 +110,10 @@ export const cleverCwd2 = /* #__PURE__ */ Object.assign({"./modules/a.ts": () =>



});
export const customBase = /* #__PURE__ */ Object.assign({"modules/a.ts": () => import("./modules/a.ts"),"modules/b.ts": () => import("./modules/b.ts"),"modules/index.ts": () => import("./modules/index.ts"),"sibling.ts": () => import("./sibling.ts")});
export const customRootBase = /* #__PURE__ */ Object.assign({"a.ts": () => import("../fixture-b/a.ts"),"b.ts": () => import("../fixture-b/b.ts"),"index.ts": () => import("../fixture-b/index.ts")

});
"
`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,9 @@ export const cleverCwd2 = import.meta.glob([
'../fixture-b/*.ts',
'!**/index.ts',
])

export const customBase = import.meta.glob('**/*.ts', { base: './' })
sapphi-red marked this conversation as resolved.
Show resolved Hide resolved

export const customRootBase = import.meta.glob('**/*.ts', {
base: '/fixture-b',
})
38 changes: 38 additions & 0 deletions packages/vite/src/node/__tests__/plugins/importGlob/parse.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,28 @@ describe('parse positives', async () => {
`)
})

it('options with base', async () => {
expect(
await run(`
import.meta.glob('**/dir/*.md', {
base: './path/to/base'
})
`),
).toMatchInlineSnapshot(`
[
{
"globs": [
"**/dir/*.md",
],
"options": {
"base": "./path/to/base",
},
"start": 5,
},
]
`)
})

it('object properties - 1', async () => {
expect(
await run(`
Expand Down Expand Up @@ -376,4 +398,20 @@ describe('parse negatives', async () => {
'[Error: Vite is unable to parse the glob options as the value is not static]',
)
})

it('options base', async () => {
expect(
await runError('import.meta.glob("./*.js", { base: 1 })'),
).toMatchInlineSnapshot(
'[Error: Expected glob option "base" to be of type string, but got number]',
)
expect(
await runError('import.meta.glob("./*.js", { base: "foo" })'),
).toMatchInlineSnapshot(
'[Error: Invalid glob: "foo/*.js" (resolved: "foo/*.js"). It must start with \'/\' or \'./\']',
)
expect(
await runError('import.meta.glob("./*.js", { base: "!/foo" })'),
).toMatchInlineSnapshot('[Error: Option "base" cannot start with "!"]')
})
})
27 changes: 23 additions & 4 deletions packages/vite/src/node/plugins/importMetaGlob.ts
sapphi-red marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ const knownOptions = {
import: ['string'],
exhaustive: ['boolean'],
query: ['object', 'string'],
base: ['string'],
sapphi-red marked this conversation as resolved.
Show resolved Hide resolved
}

const forceDefaultAs = ['raw', 'url']
Expand Down Expand Up @@ -162,6 +163,10 @@ function parseGlobOptions(
}
}

if (opts.base && opts.base[0] === '!') {
throw err('Option "base" cannot start with "!"', optsStartIndex)
}

if (typeof opts.query === 'object') {
for (const key in opts.query) {
const value = opts.query[key]
Expand Down Expand Up @@ -306,7 +311,9 @@ export async function parseImportGlob(
}

const globsResolved = await Promise.all(
globs.map((glob) => toAbsoluteGlob(glob, root, importer, resolveId)),
globs.map((glob) =>
toAbsoluteGlob(glob, root, importer, resolveId, options.base),
),
)
const isRelative = globs.every((i) => '.!'.includes(i[0]))

Expand Down Expand Up @@ -416,15 +423,24 @@ export async function transformGlobImport(
throw new Error(
"In virtual modules, all globs must start with '/'",
)
const filePath = `/${relative(root, file)}`
return { filePath, importPath: filePath }
const importPath = `/${relative(root, file)}`
const filePath = options.base
? `${relative(posix.join(root, options.base), file)}`
: importPath
return { filePath, importPath }
bluwy marked this conversation as resolved.
Show resolved Hide resolved
}

let importPath = relative(dir, file)
if (importPath[0] !== '.') importPath = `./${importPath}`

let filePath: string
if (isRelative) {
if (options.base) {
const resolvedBasePath = options.base[0] === '/' ? root : dir
filePath = relative(
posix.join(resolvedBasePath, options.base),
file,
)
} else if (isRelative) {
filePath = importPath
} else {
filePath = relative(root, file)
Expand Down Expand Up @@ -544,6 +560,7 @@ export async function toAbsoluteGlob(
root: string,
importer: string | undefined,
resolveId: IdResolver,
base?: string,
): Promise<string> {
let pre = ''
if (glob[0] === '!') {
Expand All @@ -552,6 +569,8 @@ export async function toAbsoluteGlob(
}
root = globSafePath(root)
const dir = importer ? globSafePath(dirname(importer)) : root
if (base && base.startsWith('./')) return pre + posix.join(dir, base, glob)
glob = base ? posix.join(base, glob) : glob
bluwy marked this conversation as resolved.
Show resolved Hide resolved
if (glob[0] === '/') return pre + posix.join(root, glob.slice(1))
if (glob.startsWith('./')) return pre + posix.join(dir, glob.slice(2))
if (glob.startsWith('../')) return pre + posix.join(dir, glob)
Expand Down
4 changes: 4 additions & 0 deletions packages/vite/types/importGlob.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ export interface ImportGlobOptions<
* @default false
*/
exhaustive?: boolean
/**
* Base path to resolve relative paths.
*/
base?: string
}

export type GeneralImportGlobOptions = ImportGlobOptions<boolean, string>
Expand Down
12 changes: 12 additions & 0 deletions playground/glob-import/__tests__/glob-import.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,12 @@ const relativeRawResult = {
},
}

const baseRawResult = {
'baz.json': {
msg: 'baz',
},
}

test('should work', async () => {
await withRetry(async () => {
const actual = await page.textContent('.result')
Expand Down Expand Up @@ -247,3 +253,9 @@ test('subpath imports', async () => {
test('#alias imports', async () => {
expect(await page.textContent('.hash-alias-imports')).toMatch('bar foo')
})

test('import base glob raw', async () => {
expect(await page.textContent('.result-base')).toBe(
JSON.stringify(baseRawResult, null, 2),
)
})
20 changes: 20 additions & 0 deletions playground/glob-import/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ <h2>#alias imports</h2>
<pre class="hash-alias-imports"></pre>
<h2>In package</h2>
<pre class="in-package"></pre>
<h2>Base</h2>
<pre class="result-base"></pre>

<script type="module" src="./dir/index.js"></script>
<script type="module">
Expand Down Expand Up @@ -167,3 +169,21 @@ <h2>In package</h2>
<script type="module">
import '@vitejs/test-import-meta-glob-pkg'
</script>

<script type="module">
const baseModules = import.meta.glob('*.json', {
query: '?raw',
eager: true,
import: 'default',
base: './dir',
})
const globBase = {}
Object.keys(baseModules).forEach((key) => {
globBase[key] = JSON.parse(baseModules[key])
})
document.querySelector('.result-base').textContent = JSON.stringify(
globBase,
null,
2,
)
</script>