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 all 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('./base/dir/foo.js'),
'./dir/bar.js': () => import('./base/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,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")})"
`;
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.

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.

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.

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
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: Option \"base\" must start with '/', './' or '../', but got \"foo\"]",
)
expect(
await runError('import.meta.glob("./*.js", { base: "!/foo" })'),
).toMatchInlineSnapshot('[Error: Option "base" cannot start with "!"]')
})
})
64 changes: 58 additions & 6 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,21 @@ function parseGlobOptions(
}
}

if (opts.base) {
if (opts.base[0] === '!') {
throw err('Option "base" cannot start with "!"', optsStartIndex)
} else if (
opts.base[0] !== '/' &&
!opts.base.startsWith('./') &&
!opts.base.startsWith('../')
) {
throw err(
`Option "base" must start with '/', './' or '../', but got "${opts.base}"`,
optsStartIndex,
)
}
}

if (typeof opts.query === 'object') {
for (const key in opts.query) {
const value = opts.query[key]
Expand Down Expand Up @@ -306,7 +322,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 +430,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:

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:

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,14 +578,32 @@ export async function toAbsoluteGlob(
root: string,
importer: string | undefined,
resolveId: IdResolver,
base?: string,
): Promise<string> {
let pre = ''
if (glob[0] === '!') {
pre = '!'
glob = glob.slice(1)
}
root = globSafePath(root)
const dir = importer ? globSafePath(dirname(importer)) : root
let dir
if (base) {
if (base.startsWith('/')) {
dir = posix.join(root, base)
} else {
dir = posix.resolve(
importer ? globSafePath(dirname(importer)) : root,
base,
)
}
} else {
dir = importer ? globSafePath(dirname(importer)) : root
}

if (base && glob[0] === '@') {
const withoutAlias = /\/.+$/.exec(glob)?.[0]
return pre + posix.join(root, base, withoutAlias || glob)
}
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>