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

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

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.

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.

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.

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

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

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

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 /.

Thanks, I've made the changes below.
f6c657d

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

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:

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.

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

@hakshu25 just a reminder about this PR. I'd love to help get it reviewed and merged. Will you be able to take a look at this comment still?

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.

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',
})

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.

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.

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.