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

Find less file path match error in npm package #4220

Open
treeFuck opened this issue Aug 25, 2023 · 1 comment
Open

Find less file path match error in npm package #4220

treeFuck opened this issue Aug 25, 2023 · 1 comment
Labels

Comments

@treeFuck
Copy link

To reproduce:

The code structure is like this:

my project depends on [email protected] and lessPkg2
lessPkg2 depends on [email protected]

When I try to use webpack to build my project, I found that when the lessPkg2 looks for [email protected], it will match [email protected]

Breakpoint debugging webpack, I found the loadFile function in this file(node_modules/less/lib/less-node/file-manager.js),It will only match two paths:

  • currentDirectory: The path of lessPkg2
  • '·': The node_modules directory of my project

As a result, lessPkg2 cannot find [email protected] under its own node_modules, but can only find [email protected] under node_modules of my project。

Expected behavior:
I hope that when a less file in npm package is looking for a less file in another npm package, it will first search in its own node_modules directory. If it cannot be found,it will search in the parent directory until it matches up to the root path of the project.

When I debug [email protected] source code, I found the same error, But it has been fixed in this issue

When vite is looking for module dependencies, it will recurse upwards to the node_modules directory under the directory, see the code here

I think there should be a consistent ability here.

Environment information:

  • less version: v4.2.0
  • nodejs version: v16.14.2
  • operating system: macos

other
English is not good, use translation software to write, forgive me.

@treeFuck treeFuck added the bug label Aug 25, 2023
@ARRY7686
Copy link

ARRY7686 commented Oct 4, 2024

Hello,

I’ve reviewed the issue and have an approach that could resolve this. The problem seems to lie in the loadFile function, which doesn’t recursively search for files in nested node_modules directories. To fix this, we can implement a recursive search mechanism that starts from the current directory and moves upwards, checking each parent directory’s node_modules folder for the appropriate dependency. This approach is similar to how Vite handled a similar issue.

Proposed Solution:
Modify the loadFile function to recursively search for the target file by checking each parent directory’s node_modules until the root directory is reached.
This ensures that Less will first look for dependencies in the node_modules of the package that depends on them, and only then move upwards to other directories.
Here’s a brief version of the updated logic for the loadFile function:

function loadFile(filePath, currentDirectory, options, environment, callback) {
    function recursiveFind(filePath, currentDirectory) {
        const resolvedPath = path.resolve(currentDirectory, 'node_modules', filePath);
        
        if (fs.existsSync(resolvedPath)) {
            return resolvedPath;
        }

        const parentDirectory = path.resolve(currentDirectory, '..');
        if (parentDirectory === currentDirectory) return null;

        return recursiveFind(filePath, parentDirectory);
    }

    const file = recursiveFind(filePath, currentDirectory);
    
    if (file) {
        callback(null, {
            filename: file,
            contents: fs.readFileSync(file, 'utf-8')
        });
    } else {
        callback(new Error(`File not found: ${filePath}`));
    }
}

This should fix the issue where packages were incorrectly resolving dependencies from the top-level node_modules.

Can you please assign me this issue and mark it as Hacktoberfest? I’d love to contribute this fix during Hacktoberfest. Let me know your thoughts!

Thank you!

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

No branches or pull requests

2 participants