Skip to content
This repository has been archived by the owner on Apr 6, 2023. It is now read-only.

feat(kit): support followSymbolicLinks option for resolveFiles #6240

Merged
merged 4 commits into from
Aug 2, 2022

Conversation

Diizzayy
Copy link
Member

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

This PR allows module authors to opt-out of following symbolic links when using resolveFiles, as there are many cases where this current default isn't the desired.

@netlify
Copy link

netlify bot commented Jul 29, 2022

βœ… Deploy Preview for nuxt3-docs canceled.

Name Link
πŸ”¨ Latest commit 2a6f905
πŸ” Latest deploy log https://app.netlify.com/sites/nuxt3-docs/deploys/62e7932876410a000853184c

@pi0 pi0 changed the title fix(kit): improve resolveFiles by allowing opt-out of followSymbolicLinks feat(kit): support followSymbolicLinks option for resolveFiles Jul 29, 2022
@@ -151,7 +151,7 @@ async function isDirectory (path: string) {
return (await fsp.lstat(path)).isDirectory()
}

export async function resolveFiles (path: string, pattern: string | string[]) {
const files = await globby(pattern, { cwd: path, followSymbolicLinks: true })
export async function resolveFiles (path: string, pattern: string | string[], followSymbolicLinks: boolean = true) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please make 3rd argument an object { followSymbolicLinks }

@pi0
Copy link
Member

pi0 commented Jul 29, 2022

Thanks for PR @Diizzayy. In the future, please consider opening an issue describing improvement and enhancement πŸ™πŸΌ Also what is the current use case you have for this utility for skipping link resolution? Knowing context, can help to improve utilities and be aware of usecases.

@Diizzayy
Copy link
Member Author

In the future, please consider opening an issue describing improvement and enhancement πŸ™πŸΌ

Sorry about that, I had begun writing an issue when I realized that this was the underlying cause.
I'll be sure to keep this in mind going forward. I jumped the gun here.

Also what is the current use case you have for this utility for skipping link resolution

@pi0 When using pnpm workspaces which uses symlinks between packages, you'll find that based on how the resolveFiles utility is used in a module, it's default behavior ( following symlinks ) can result in scanning deeply nested files in node_modules that match the pattern ( which may not be the desired ). This can also lead to an almost infinite loop of resolving files when a symlinked package depends on another that also contains matching files (which is where I encountered it).

packages/kit/src/resolve.ts Outdated Show resolved Hide resolved
packages/kit/src/resolve.ts Outdated Show resolved Hide resolved
@pi0 pi0 merged commit 8efaad4 into nuxt:main Aug 2, 2022
@Diizzayy Diizzayy deleted the patch branch August 2, 2022 12:29
@pi0 pi0 mentioned this pull request Aug 5, 2022
@danielroe danielroe added the 3.x label Jan 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants