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.

Sign up for GitHub

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

Jump to bottom

feat: add base option for import.meta.glob #18510

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

feat: add base option for import.meta.glob #18510

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
fd262c0
feat: parse base path in import.meta.glob
hakshu25 Oct 29, 2024
09a3e9c
feat: add support for custom base path in import.meta.glob
hakshu25 Oct 29, 2024
6b17fd7
test: add integration test for base option of glob-import
hakshu25 Oct 29, 2024
ae89d14
feat: add validation to check if the base option doesn't start with '!'
hakshu25 Oct 30, 2024
5db0640
fix: resolved relative paths to options.base
hakshu25 Oct 31, 2024
f6d73df
docs: add usage of base path for glob import
hakshu25 Oct 31, 2024
f206609
docs: remove one example
sapphi-red Nov 1, 2024
7f0f5e7
fix: ensure glob import keys start with './' when using base option
hakshu25 Nov 2, 2024
1cd16f3
docs: fix glob import paths to use base directory
hakshu25 Nov 2, 2024
51e76be
fix: ensure glob import keys start with './' for base directory in gl…
hakshu25 Nov 2, 2024
b07d814
feat: support alias in glob with options.base
hakshu25 Nov 5, 2024
2ec1175
feat: support relative glob in virtual modules
hakshu25 Nov 5, 2024
76dc01e
test: fix test to treat as base instead of prefix
hakshu25 Nov 6, 2024
500cb14
docs: fix glob import example to treat as base instead of prefix
hakshu25 Nov 6, 2024
f6c657d
fix: changed to update dir to glob to act as base instead of prefix
hakshu25 Nov 7, 2024
1a26072
test: fix e2e test glob in base option of import glob
hakshu25 Nov 8, 2024
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
View file
Open in desktop
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('./base/dir/foo.js'),
'./dir/bar.js': () => import('./base/dir/bar.js'),
}
```

### Glob Import Caveats

Note that:
Expand Down
17 changes: 16 additions & 1 deletion packages/vite/src/node/__tests__/plugins/importGlob/__snapshots__/fixture.spec.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ 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")

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

});
"
`;
Expand Down Expand Up @@ -106,11 +113,19 @@ 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")

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

});
"
`;

exports[`fixture > virtual modules 1`] = `
"/* #__PURE__ */ Object.assign({"/modules/a.ts": () => import("/modules/a.ts"),"/modules/b.ts": () => import("/modules/b.ts"),"/modules/index.ts": () => import("/modules/index.ts")})
/* #__PURE__ */ Object.assign({"/../fixture-b/a.ts": () => import("/../fixture-b/a.ts"),"/../fixture-b/b.ts": () => import("/../fixture-b/b.ts"),"/../fixture-b/index.ts": () => import("/../fixture-b/index.ts")})"
/* #__PURE__ */ Object.assign({"/../fixture-b/a.ts": () => import("/../fixture-b/a.ts"),"/../fixture-b/b.ts": () => import("/../fixture-b/b.ts"),"/../fixture-b/index.ts": () => import("/../fixture-b/index.ts")})
/* #__PURE__ */ Object.assign({"./a.ts": () => import("/modules/a.ts"),"./b.ts": () => import("/modules/b.ts"),"./index.ts": () => import("/modules/index.ts")})"
`;
10 changes: 10 additions & 0 deletions packages/vite/src/node/__tests__/plugins/importGlob/fixture-a/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,13 @@ export const cleverCwd2 = import.meta.glob([
'../fixture-b/*.ts',
'!**/index.ts',
])

export const customBase = import.meta.glob('./**/*.ts', { base: './' })

export const customRootBase = import.meta.glob('./**/*.ts', {
base: '/fixture-b',
})

export const aliasBase = import.meta.glob('@modules/*.ts', {
base: '/fixture-a/modules',
})
Comment on lines +77 to +79
Copy link
Member

Choose a reason for hiding this comment

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

To test this correctly, this resolveId function needs to have an alias feature supported.

vite/packages/vite/src/node/__tests__/plugins/importGlob/fixture.spec.ts

Line 11 in 500cb14

const resolveId = (id: string) => id 

const resolveId = (id) => {
  if (id.startsWith('@modules/')) {
    return normalizePath(path.resolve(root, 'modules', id.slice('@modules/'.length)))
  }
  return id
}

I expect the output to be same with

import.meta.glob('@modules/*.ts')

if the alias works like above.

I think we should also test:

import.meta.glob('./*.ts', {
  base: '@modules',
})

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 output to be same with

import.meta.glob('@modules/*.ts')

You're right. I was actually suggesting a different output before (#18510 (comment)). Since base is now where we resolve the globs from, but aliases will always return a root-relative key, that might be one of the downsides if treating base this way I think. Unless we want to make base unique where, if it's specified, we'll resolve the keys relative to it?

I think we should also test:

import.meta.glob('./*.ts', {
  base: '@modules',
})

The implementation I had in mind when I added my reviews before is that base: '@modules' is a little tricky to support, and we can only support those that starts with / or ./ or ../. Since we're trying to figure out the dir to resolve from, and resolving alias would have to involve resolveId, but I don't think it handles resolving dirs well, only files.

Copy link
Member

Choose a reason for hiding this comment

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

Since we're trying to figure out the dir to resolve from, and resolving alias would have to involve resolveId, but I don't think it handles resolving dirs well, only files.

We are passing glob to resolveId here.

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

Lines 594 to 598 in 500cb14

const resolved = normalizePath(
(await resolveId(glob, importer, {
custom: { 'vite:import-glob': { isSubImportsPattern } },
})) || glob,
)

Maybe the dirs can be resoved by something like resolveId(base + '*').slice(0, -'*'.length)?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm yeah I guess if globs like @alias/*.ts can already be resolved here, then base should be able to do the same here too. Though if it's harder to get it right I was thinking it could be implemented later in the future. But fine by me either ways.

1 change: 1 addition & 0 deletions packages/vite/src/node/__tests__/plugins/importGlob/fixture.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ describe('fixture', async () => {
const code = [
"import.meta.glob('/modules/*.ts')",
"import.meta.glob(['/../fixture-b/*.ts'])",
"import.meta.glob(['./*.ts'], { base: '/modules' })",
].join('\n')
expect(
(
Expand Down
38 changes: 38 additions & 0 deletions packages/vite/src/node/__tests__/plugins/importGlob/parse.spec.ts
View file
Open in desktop
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 "!"]')
})
})
42 changes: 37 additions & 5 deletions packages/vite/src/node/plugins/importMetaGlob.ts
View file
Open in desktop
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 @@ -412,19 +419,35 @@ export async function transformGlobImport(

const resolvePaths = (file: string) => {
if (!dir) {
Copy link
Member

Choose a reason for hiding this comment

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

This specific dir like my other comment may need to be updated too. In the code above it leads to:

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

Lines 376 to 383 in ce0eec9

const dir = isVirtual ? undefined : dirname(id)
const matches = await parseImportGlob(
code,
isVirtual ? undefined : id,
root,
resolveId,
logger,
)

It should also consider base like shown in my comment. However, we don't know about base until parseImportGlob() is called.

Maybe a refactor here could be to remove this dir entirely. parseImportGlob can return and we access from here instead:

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

Lines 392 to 393 in ce0eec9

matches.map(
async ({ globsResolved, isRelative, options, index, start, end }) => {

e.g.

async ({ globsResolved, isRelative, options, index, start, end, dir }) => {

Since we only need the dir within that callback. Free to explore other options to refactor this too though.

Copy link
Author

Choose a reason for hiding this comment

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

It would be great to make the dir reusable. I'll try refactoring it.

if (isRelative)
if (!options.base && isRelative)
Copy link
Member

Choose a reason for hiding this comment

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

With the updates only to dir in my comments, I believe we can then revert this part, from line 422 to 450, as I think it should automatically work then.

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)}`
let filePath = options.base
? `${relative(posix.join(root, options.base), file)}`
: importPath
if (options.base && filePath[0] !== '.') {
filePath = `./${filePath}`
}
return { filePath, importPath }
}

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,
)
if (filePath[0] !== '.') filePath = `./${filePath}`
if (options.base[0] === '/') {
importPath = `/${relative(root, file)}`
}
} else if (isRelative) {
filePath = importPath
} else {
filePath = relative(root, file)
Expand Down Expand Up @@ -544,6 +567,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 +576,14 @@ export async function toAbsoluteGlob(
}
root = globSafePath(root)
const dir = importer ? globSafePath(dirname(importer)) : root
if (base) {
if (base.startsWith('./')) return pre + posix.join(dir, base, glob)
if (glob[0] === '@') {
const withoutAlias = /\/.+$/.exec(glob)?.[0]
return pre + posix.join(root, base, withoutAlias || glob)
}
}
glob = base ? posix.join(base, glob) : glob
Copy link
Member

Choose a reason for hiding this comment

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

Since base is now where we resolve globs from, I think what we should do instead is to update the dir here. For example, base should be used as dir if it's defined, otherwise we falllback to the existing logic. Like:

let dir
if (base) {
  if (base.startsWith('/')) { // root-relative
    dir = posix.join(root, base)
  } else { // assume relative path from the current file
    dir = posix.resolve(importer, base)
  }
} else {
  dir = importer ? globSafePath(dirname(importer)) : root
}

We shouldn't need to update or return the glob ourselves. That would be like prefixing.

This assumes that base can only be ./foo (relative to the current file) or /src/foo (relative from the root), which I think should be sufficient for an initial implementation. We might want to validate in parseGlobOption that it can only start with ./, ../. or /.

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've made the changes below.
f6c657d

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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>