unplugin-vue-components icon indicating copy to clipboard operation
unplugin-vue-components copied to clipboard

When using `tinyglobby` get wrong components files order

Open SoulLyoko opened this issue 10 months ago • 15 comments

Describe the bug

The latest version uses tinyglobby and gets the wrong order: Image

v28.0.0 uses fast-glob to get the correct order: Image

What I want to get is that web/src/components/TheFooter.vue overrides layer-shared/src/components/TheFooter.vue.

Reproduction

no repo

System Info

System:
    OS: Windows 10 10.0.19045
    CPU: (12) x64 Intel(R) Core(TM) i7-8700 CPU @ 3.20GHz
    Memory: 8.56 GB / 31.87 GB
  Binaries:
    Node: 22.10.0 - C:\Program Files\nodejs\node.EXE     
    Yarn: 1.22.22 - C:\Program Files\nodejs\yarn.CMD     
    npm: 10.9.0 - C:\Program Files\nodejs\npm.CMD        
    pnpm: 9.15.5 - C:\Program Files\nodejs\pnpm.CMD      
  Browsers:
    Edge: Chromium (133.0.3065.69)
    Internet Explorer: 11.0.19041.4355

Used Package Manager

npm

Validations

  • [x] Follow our Code of Conduct
  • [x] Read the Contributing Guide.
  • [x] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
  • [x] Check that this is a concrete bug. For Q&A, please open a GitHub Discussion instead.
  • [x] The provided reproduction is a minimal reproducible of the bug.

SoulLyoko avatar Feb 25 '25 09:02 SoulLyoko

However, according to the fast-glob docs, results are returned in arbitrary order.

ilyaliao avatar Jun 20 '25 12:06 ilyaliao

I also need a deterministic method for overwriting, whereby one directory should take precedence over the other.

samuelwei avatar Sep 25 '25 16:09 samuelwei

I added a sort function where you can add your logic to sort your components, the test added in the PR using simple ascending/descending sorts, I guess with this callback applied before generating the output record should be fine for your use case, maybe we can use asc sort by default.

We can also iterate later using the dirs order to apply 2 sorts, first using the dirs order and then by name for each dirs entry.

userquin avatar Sep 25 '25 19:09 userquin

However, according to the fast-glob docs, results are returned in arbitrary order.

This is not fine if the arbitrary order breaks for example the dirs/globs order. Looks like the globs being sorted in ascending order.

I'm going to check tinyglobby, looks like a bug to me.

userquin avatar Sep 25 '25 19:09 userquin

https://github.com/SuperchupuDev/tinyglobby/issues/166

userquin avatar Sep 25 '25 19:09 userquin

I added a sort function where you can add your logic to sort your components, the test added in the PR using simple ascending/descending sorts, I guess with this callback applied before generating the output record should be fine for your use case, maybe we can use asc sort by default.

We can also iterate later using the dirs order to apply 2 sorts, first using the dirs order and then by name for each dirs entry.

This would be extremely helpful, I was looking for something like this in the source code this evening and tried to setup a custom resolver to replace the glob / dir feature, but it’s not really good, especially for developing where files are added, renamed and removed. Really appreciate your work on a fix

samuelwei avatar Sep 25 '25 19:09 samuelwei

I'll try adding a new option at tinyglobby, unimport and unplugin-auto-import may have the same issue.

/cc @danielroe @antfu

userquin avatar Sep 25 '25 20:09 userquin

@SamuelWei I'm checking this: https://github.com/SuperchupuDev/tinyglobby/issues/166#issuecomment-3335809326

userquin avatar Sep 25 '25 20:09 userquin

PR sent to tinyglobby 🤞 https://github.com/SuperchupuDev/tinyglobby/pull/167

userquin avatar Sep 25 '25 22:09 userquin

I'll try adding a new option at tinyglobby, unimport and unplugin-auto-import may have the same issue.

/cc @danielroe @antfu

Disadvantage of adding it to tinyglobby, you can not replace this dependency with another glob lib in the future. Is there a big performance diff. in running the glob for each dir/glob speratly vs. all at once? If not, running the glob search sequentially could also result in a deterministic output.

samuelwei avatar Sep 26 '25 08:09 samuelwei

Yeah, since we also have a watcher, we need to sort the files at least when adding files, maybe we need some helper function at tiny-globby, it is using picomatch or maybe just add some logic there here: we only need the matched patterns and the picomatch options.

Will try another approach, running N scans (1 per pattern/glob) instead 1 global. Maybe we can also move sync to async scan, I need to review the code and the perf. We still have the same issue about the watcher when adding new files.

I also need to check unimport and unplugin-auto-import (not the same problem, since unimport has an order where you can apply some priority => not sure if it is a nuxt thingy) => unimport type => usage https://github.com/elk-zone/elk/blob/main/nuxt.config.ts#L90-L94

userquin avatar Sep 26 '25 10:09 userquin

@SamuelWei https://github.com/SuperchupuDev/tinyglobby/pull/168 (new helper function at tinyglobby abstracting picomatch usage, this way, if tinyglobby changes its internal matcher, this repo should be also fine => don't require to add picomatch or switch to xxxmatch when tinyglob changes it internally)

I'm working on the PR to include the abstraction using Generator, here some work on my local, the sortFilesByGlobPrecedence should go to src/core/fs/glob.ts, and should use the new compileGlob helper function at https://github.com/SuperchupuDev/tinyglobby/pull/168:

// src/core/context.ts
export class Context {
  // other entries
  private* sortFilesByGlobPrecedence(files: Iterable<string>): Generator<string, undefined, void> {
    const processedFiles = new Set<string>()
    const allFiles = Array.from(files)

    const { globs, ...rest } = this.options
    for (const glob of globs) {
      const isMatch = picomatch(glob, rest)

      for (const file of allFiles) {
        if (!processedFiles.has(file) && isMatch(file)) {
          processedFiles.add(file)
          yield file
        }
      }
    }
  }

  private* preparePaths(): Generator<string, undefined, void> {
    if (this.options.sortByGlob) {
      yield* this.sortFilesByGlobPrecedence(this._componentPaths)
    }
    else {
      yield* this._componentPaths
    }
  }

  private* applyPathsSorting(files: Iterable<string>): Generator<string, undefined, void> {
    const generator = this.options.sort
    if (generator) {
      const {
        root,
        globs,
        globsExclude,
        resolvedDirs,
      } = this.options
      yield* generator({
        root,
        globs,
        globsExclude,
        resolvedDirs,
      }, Array.from(files))
    }
    else {
      yield* files
    }
  }

  private updateComponentNameMap() {
    this._componentNameMap = {}

    const { prefix, excludeNames, allowOverrides } = this.options
    for (const path of this.applyPathsSorting(this.preparePaths())) {
      const fileName = getNameFromFilePath(path, this.options)
      const name = prefix
        ? `${pascalCase(prefix)}${pascalCase(fileName)}`
        : pascalCase(fileName)
      if (isExclude(name, excludeNames)) {
        debug.components('exclude', name)
        continue
      }
      if (this._componentNameMap[name] && !allowOverrides) {
        console.warn(`[unplugin-vue-components] component "${name}"(${path}) has naming conflicts with other components, ignored.`)
        continue
      }

      this._componentNameMap[name] = {
        as: name,
        from: path,
      }
    }
  }
}

and here the new sort and sortByGlob options:

// src/types.ts
/**
 * Plugin options.
 */
export interface Options {
  // other options
  /**
   * Enable `globs` sort for scanned files.
   *
   * `unplugin-vue-components` using `tinyglobby` to scan files, which is non-deterministic.
   */
  sortByGlob?: true

  /**
   * Generator to provide files before preparing the components.
   *
   * @param options The resolved options.
   * @param files The scanned files.
   */
  sort?: (options: Pick<ResolvedOptions, 'root' | 'resolvedDirs' | 'globs' | 'globsExclude'>, files: string[]) => Generator<string, undefined, void>
}

userquin avatar Sep 26 '25 16:09 userquin

@SamuelWei @SoulLyoko my PR here updated using pkg-pr-new from compileGlobs PR at tinyglobby, I'm going to add pkg-pr-new also to unplugin-vue-components.

userquin avatar Sep 26 '25 20:09 userquin

@userquin Any progress on this?

samuelwei avatar Oct 27 '25 13:10 samuelwei

No, the PR here should be ready (pr moved to draft), awaiting tinyglobby: https://github.com/SuperchupuDev/tinyglobby/pull/168#issuecomment-3393041398

userquin avatar Oct 27 '25 14:10 userquin