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

Unable to resolve aliased dependency from nested directory #361

Closed
ehoogeveen-medweb opened this issue Feb 19, 2025 · 24 comments
Closed

Unable to resolve aliased dependency from nested directory #361

ehoogeveen-medweb opened this issue Feb 19, 2025 · 24 comments

Comments

@ehoogeveen-medweb
Copy link

ehoogeveen-medweb commented Feb 19, 2025

As of v3.8.0 some of my imports aren't resolving. It still happens as of v3.8.2 and doesn't seem like it's related to #359 as there are no dot directories involved.

Instead, a crucial component seems to be that the file attempting to resolve the aliased dependency must be nested a few levels deep. I'm not sure exactly what the conditions are, but I created a reproduction repo showing that it works with less nesting, but not with more.

Reproduction repo: https://github.com/ehoogeveen-medweb/unresolved-repro

To reproduce:

  • Clone the repo
  • Run npm ci
  • Run npx eslint .

Expected result:
No errors, as with version v3.7.0.

Actual result:

  • src/working.ts gives no errors
  • src/nested/broken.ts gives error Unable to resolve path to module 'ext/ua-parser.js'
@carlocorradini
Copy link
Contributor

I'll take a look ASAP

@carlocorradini
Copy link
Contributor

Ok, I discovered the problem: src/nested/tsconfig.json has file broken.ts defined in the include property. According to TypeScript documentation, the include property should only contain glob patterns (a problem I've discovered while working with references):

Specifies a list of glob patterns that match files to be included in compilation. If no 'files' or 'include' property is present in a tsconfig.json, the compiler defaults to including all files in the containing directory and subdirectories except those specified by 'exclude'. Requires TypeScript version 2.0 or later.

Therefore, replace "include": ["broken.ts"] with "files": ["broken.ts"] or use a glob pattern "include": ["**/*.ts"].

I'm not sure if @JounQin wants to support the strange behavior in which we must check if each entry in include is a dynamic pattern using the isDynamicPattern function from tinyglobby. If true, expand; otherwise, treat it as a file entry, prepending the tsconfig.json dirname

@ehoogeveen-medweb
Copy link
Author

ehoogeveen-medweb commented Feb 19, 2025

Ah, that's an artifact of me trying to get a minimal reproduction. However it also seems to fail with:

  • "include": ["*.ts"]
  • "include": ["*"]
  • "include": ["**/*.ts"]
  • "include": ["**/*"]

@ehoogeveen-medweb
Copy link
Author

But you are correct that it works using files in the nested case; include is required to reproduce the problem.

@JounQin
Copy link
Collaborator

JounQin commented Feb 19, 2025

I think broken.ts is also a valid glob pattern, isn't it? tinyglobby doesn't support it?

See fast-glob example:

https://github.com/mrmlnc/fast-glob?tab=readme-ov-file#synchronous

@carlocorradini
Copy link
Contributor

Unfortunately, I've discovered the issue... it's something related to tinyglobby that does not work correctly when the ignore array contains paths that begin with ../ (in your example, ../../dist).
@Jason3S Discovered it and wrote the comment SuperchupuDev/tinyglobby#90 (comment)
@JounQin We are blocked until tinyglobby resolves the issue 😥
@SuperchupuDev You can use https://github.com/ehoogeveen-medweb/unresolved-repro as tester

@carlocorradini
Copy link
Contributor

@JounQin I can confirm that if a pattern is not dynamic (i.e. a real path to a file) and ignore contains ../, tinyglobby always returns an empty array. Same as previous

@SuperchupuDev
Copy link

SuperchupuDev commented Feb 19, 2025

@carlocorradini so the whole issue has to do with ../ and not the dot option? i thought the two were two different issues. checking it out right now

edit: didn't notice this was a different issue, my bad

@carlocorradini
Copy link
Contributor

@SuperchupuDev AFAIK they are separated:

  • dot is correctly handled in the new version where dot directories are loaded only if the property is set to true. In the old version it looks like it was silently ignored and dot directories are loaded by default.
  • ../ in ignore is the real problem. If it's present in the ignore property tinyglobby always returns an empty array.

@SuperchupuDev
Copy link

SuperchupuDev commented Feb 19, 2025

okay! although this issue is very odd to me. not just that i can't locally reproduce it myself (by doing the following)

 await glob(['/home/meow/code/fdir/src/*'], {
    debug: true,
    ignore: ['node_modules/**', '../../test'],
    expandDirectories: false
  })

it's also that afaik the code that handles ../ shouldn't have changed between versions. unfortunately i'm not sure what to do with https://github.com/ehoogeveen-medweb/unresolved-repro, i have never used this project since i normally use biome and there is no list of steps

@carlocorradini
Copy link
Contributor

Try removing expandDirectories, add absolute: true and set cwd: ?.
Ignore: ignore: ['**/node_modules/**', '../../test']

@carlocorradini
Copy link
Contributor

@SuperchupuDev The steps to reproduce are in the first comment of this issue 👍

@SuperchupuDev
Copy link

nevermind i just managed to locally repro it

Try removing expandDirectories, add absolute: true and set cwd: ?. Ignore: ignore: ['**/node_modules/**', '../../test']

no need, issue happens regardless

@carlocorradini
Copy link
Contributor

@SuperchupuDev Awesome

@SuperchupuDev
Copy link

i think i got a fix! can y'all try to see if it's still an issue with this preview release by adding the following to your package.json?

"resolutions": {
  "tinyglobby": "https://pkg.pr.new/tinyglobby@d5fb84c"
},

@carlocorradini
Copy link
Contributor

@SuperchupuDev What was the issue?
I'll try tomorrow 👍

@SuperchupuDev
Copy link

SuperchupuDev commented Feb 19, 2025

@carlocorradini 0.2.11 introduced some more matching when crawling to avoid crawling more directories than needed. however by using ignore like that it tries to match i.e. ../.. to src/** which obviously returns false. as such the parent directory would not get crawled so no crawling would be done. in SuperchupuDev/tinyglobby#91 i just made that matcher always return true if the crawling pattern only has parent directories, i.e. .., ../.. etc

@carlocorradini
Copy link
Contributor

@SuperchupuDev Thanks 👍

@ehoogeveen-medweb
Copy link
Author

Using that preview release fixes the issue for me!

I tried it both on the reproduction repository and on my actual projects: All my imports resolve again now :)

By the way, the npm equivalent of resolutions is overrides:

"overrides": {
  "tinyglobby": "https://pkg.pr.new/tinyglobby@d5fb84c"
}

@carlocorradini
Copy link
Contributor

@ehoogeveen-medweb Awesome thanks for trying it before me :)
@SuperchupuDev I think we can confirm it as a fix?

@SuperchupuDev
Copy link

probably! will test some more to make sure no other inputs can cause it, look into the dot issue (to see whether the behavior in 0.2.10 or 0.2.11 was more correct) and release the fix 👍

@SuperchupuDev
Copy link

0.2.12 has been released with the fix 👍

@carlocorradini
Copy link
Contributor

tinyglobby updated: included in PR #360

@carlocorradini
Copy link
Contributor

@JounQin Close this thanks to PR #360

@JounQin JounQin closed this as completed Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants