next.js icon indicating copy to clipboard operation
next.js copied to clipboard

fix: ensure absolute paths are handled correctly with `--file` option in `next lint` command for `lint-staged` compatibility

Open lumirlumir opened this issue 6 months ago • 1 comments

Hello😊 Thanks for having attention to this issue.

What?

Fixed a bug.

The next lint command's --file option was intended to allow users to lint specific files directly by specifying their paths. However, it only worked with relative paths and did not handle absolute paths correctly. This limitation hindered the flexibility and usability of the linting process.

Furthermore, lint-staged returns absolute paths when running lint commands on staged files. Without proper handling of these absolute paths, linting was not performed correctly, leading to issues in automated linting workflows.

Why?

Environment(results by next info)

Operating System:
  Platform: win32
  Arch: x64
  Version: Windows 11 Home
  Available memory (MB): 16058
  Available CPU cores: 8
Binaries:
  Node: 20.12.2
  npm: N/A
  Yarn: N/A
  pnpm: N/A
Relevant Packages:
  next: 14.2.6 // Latest available version is detected (14.2.6).
  eslint-config-next: 14.2.6
  react: 18.3.1
  react-dom: 18.3.1
  typescript: 5.5.4
Next.js Config:
  output: N/A

Steps to Reproduce

  1. Make a js or jsx file like below. this makes ESLint error named no-unused-vars

    /* src/app/test.jsx */
    
    const a = 1;
    
  2. Run eslint and next lint to the same file.

    See screenshot below.

    eslint command works correctly with absolute path. but, next lint command doesn't work correctly with absolute path.

    스크린샷 2024-08-23 173447

Code Details

/* packages/next/src/cli/next-lint.ts */

// ...

const pathsToLint = (
    filesToLint.length ? filesToLint : ESLINT_DEFAULT_DIRS
  ).reduce((res: string[], d: string) => {
    const currDir = join(baseDir, d)

    if (!existsSync(currDir)) {
      return res
    }

    res.push(currDir)
    return res
  }, [])

// ...

pathsToLint

  • pathsToLint gets an empty array [] when filesToLint array is [ 'D:\\Cloud_Git\\web-blog\\src\\app\\test.jsx' ] passed by --file option from CLI.

The reduce Function

  • For each element (d) in filesToLint, it combines it with baseDir to create currDir.
  • It checks if this path exists using existsSync(currDir).
  • If the path does not exist, it does not add it to the resulting array (res), and if it does exist, it adds it.

Cause of the Issue

  • join(baseDir, d):

    • The join function is used to combine file paths and directory paths.
    • If baseDir is D:\Cloud_Git\web-blog, then currDir becomes D:\Cloud_Git\web-blog\D:\Cloud_Git\web-blog\src\app\test.jsx, creating an incorrect path.
  • existsSync(currDir):

    • With the incorrect path (D:\Cloud_Git\web-blog\D:\Cloud_Git\web-blog\src\app\test.jsx), existsSync determines that this path does not exist, so it does not add it to the res array.
    • As a result, pathsToLint ends up being an empty array [].

How? (Solution)

The code should be modified to use the file path as-is, without combining it with baseDir. If a file path is provided, it should be directly included in pathsToLint.

  • Updated Path Handling: Modified the code that processes the --file option to properly handle absolute paths. This involved ensuring that absolute paths are correctly interpreted and used by the linting command.
  • Enhanced Compatibility with lint-staged: Adjusted the linting logic to handle absolute paths returned by lint-staged, ensuring that linting works as expected in automated workflows.

The code below is the built code of the modified version.


/* node_modules/next/dist/cli/next-lint.js */

// ...

const pathsToLint = (filesToLint.length ? filesToLint : _constants.ESLINT_DEFAULT_DIRS).reduce((res, d)=>{
    const currDir = (0, _fs.existsSync)(d) ? d : (0, _path.join)(baseDir, d);

    if (!(0, _fs.existsSync)(currDir)) {
        return res;
    }
    res.push(currDir);
    return res;
}, []);

// ...


Closes: I tried to find but No related Issues. Fixes: I tried to find but No related Issues.

lumirlumir avatar Aug 23 '24 11:08 lumirlumir