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

Cannot automatically import multiple levels of nested directories under utils #2918

Open
noootwo opened this issue Dec 4, 2024 · 6 comments

Comments

@noootwo
Copy link

noootwo commented Dec 4, 2024

Environment

  • Operating System: Linux
  • Node Version: v18.20.3
  • Nitro Version: 2.10.4
  • Package Manager: [email protected]

Reproduction

https://stackblitz.com/edit/github-pxnqrn?file=server%2Futils%2Findex.ts,server%2Futils%2Ffoo%2Fbar%2Findex.ts,server%2Futils%2Ffoo%2Findex.ts,server%2Froutes%2Findex.ts

Describe the bug

Cannot automatically import multi-level of nested directories under server/utils.

In unimportversion 3.13 and earlier, auto-importing worked for two levels.
However, in unimport version 3.14, it only works for one level.

Additional context

Is automatically import of multi-level(>3) directories expected behavior?

Logs

No response

@pi0
Copy link
Member

pi0 commented Dec 4, 2024

/cc @antfu any clues? (i haven't checked anything yet but imagined you might have better context for resolution 🙏🏼 )

@noootwo
Copy link
Author

noootwo commented Dec 4, 2024

@pi0 If auto import multi-level directories is the desired behavior, I can submit a PR to fix this issue

@antfu
Copy link
Collaborator

antfu commented Dec 4, 2024

/cc @antfu any clues? (i haven't checked anything yet but imagined you might have better context for resolution 🙏🏼 )

Explaining the context:

@noootwo worked on the PRs to unimport that changed a bit of how dirs's root is resolved, which landed in [email protected] and caused some behavior change in nitro. It was because Nitro put utils/* in dirs (

...options.scanDirs.map((dir) => join(dir, "utils/*"))
) instead of utils. Previously, it was resolved as utils/**/*.{ts,js}, but now it is resolved to utils/*.{ts,js}. This issue was mainly asking about what's your original expectation of it, if you was expecting utils to resolve to nested folder, we would fix it in unimport.

My two cents:

I think we should fix that in unimport to align with the previous behavior. Otherwise it would be a breaking change to existing Nitro users anyway. Even if it was not a designed behavior (which to me it seems to expect to be deep), we could figure out a way to smooth it out and make the breakage in a major of nitro.

@noootwo
Copy link
Author

noootwo commented Dec 4, 2024

/cc @antfu any clues? (i haven't checked anything yet but imagined you might have better context for resolution 🙏🏼 )

Explaining the context:

@noootwo worked on the PRs to unimport that changed a bit of how dirs's root is resolved, which landed in [email protected] and caused some behavior change in nitro. It was because Nitro put utils/* in dirs (

...options.scanDirs.map((dir) => join(dir, "utils/*"))

) instead of utils. Previously, it was resolved as utils/**/*.{ts,js}, but now it is resolved to utils/*.{ts,js}. This issue was mainly asking about what's your original expectation of it, if you was expecting utils to resolve to nested folder, we would fix it in unimport.

My two cents:

I think we should fix that in unimport to align with the previous behavior. Otherwise it would be a breaking change to existing Nitro users anyway. Even if it was not a designed behavior (which to me it seems to expect to be deep), we could figure out a way to smooth it out and make the breakage in a major of nitro.

Correct a small error described here, previously, it was resolved as utils/* and utils/*/*.{ts,js}, so it can match the first and second levels files of the utils directory.

So if expect to match deep-level files, there were also some issues before.: )

@noootwo
Copy link
Author

noootwo commented Dec 4, 2024

So I tend to prefer maintaining the current unimport behavior, that better aligns with the intuitive understanding of the dirs option.

This means that need to change utils/* to utils/**/* in nitro.

If my understanding is off track, please correct me, thanks.

@antfu
Copy link
Collaborator

antfu commented Dec 6, 2024

Yeah I agree that only two-level nesting is confusing, it should be either one level or deep. /cc @pi0 WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants