feat: replace cosmiconfig with lilconfig + yaml to reduce dependencies

[iiroj]

This PR revives #981 by replacing cosmiconfig with lilconfig + yaml to reduce the number and size of dependencies used by lint-staged. The first PR introduced some problems (#1033) and was later reverted (#1035).

It seems lilconfig doesn't support loading configurations from a module, so that functionality was added as well.

This draft will be left for later...

[iiroj]

Ping @antonk52, it seems lilconfig doesn't support loading from a module by default; is this correct?

[codecov]

Codecov Report

Merging #1042 (67780a2) into fix-esm-config (978aa9e) will not change coverage.
The diff coverage is 100.00%.

@@               Coverage Diff                @@
##           fix-esm-config     #1042   +/-   ##
================================================
  Coverage          100.00%   100.00%           
================================================
  Files                  21        21           
  Lines                 579       583    +4     
  Branches              156       156           
================================================
+ Hits                  579       583    +4     

Impacted Files	Coverage Δ
lib/index.js	100.00% <100.00%> (ø)
lib/loadConfig.js	100.00% <100.00%> (ø)
lib/symbols.js	100.00% <100.00%> (ø)
lib/validateConfig.js	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 978aa9e...67780a2. Read the comment docs.

[antonk52]

Hey @iiroj

If I got that right and you mean ecmascript modules in terms of *.mjs files. It does not, same as cosmiconfig

[iiroj]

@antonk52 See this part in the PR:

https://github.com/okonet/lint-staged/pull/1042/files#diff-bff4b732db5b89cc001efe07fb282c50542546d6ccd1f7ee2053f7ff5afdd89dR38-R51

without it, a test will fail that tries to load config from my-lint-staged-config (a node module) with the following error:

ENOENT: no such file or directory, open '/Users/iiro/git/lint-staged/my-lint-staged-config'

[antonk52]

Thanks for the explanation. I'm currently out. I will have a good look through the PR later on

[antonk52]

Thank you for the patience. I've checked the PR. You're statement is correct about lilconfig not supporting (resolving paths for) node_modules out of the box. I want to note that the same can be said about cosmiconfig as they resolve provided paths the same using path.resolve internally. Please see the references for lilconfig and cosmiconfig.

I can see that some part of the code was modified in this PR. More specifically the resolveConfig function. If we leave it as is and use it for lilconfig the same way it is currently used for cosmiconfig the test will pass and the behaviour should remain the same.

const resolveConfig = (configPath) => {
  try {
    return require.resolve(configPath)
  } catch {
    return configPath
  }
}

const loadConfig = (configPath) => {
-  const explorer = cosmiconfig('lint-staged', {
+  const explorer = lilconfig('lint-staged', {
    searchPlaces: [
      'package.json',
      '.lintstagedrc',
      '.lintstagedrc.json',
      '.lintstagedrc.yaml',
      '.lintstagedrc.yml',
      '.lintstagedrc.js',
      '.lintstagedrc.cjs',
      'lint-staged.config.js',
      'lint-staged.config.cjs',
    ],
  })

  return configPath ? explorer.load(resolveConfig(configPath)) : explorer.search()
  //                                ^^^^^^^^^^^^^ This function call should not be removed
}

By preserving resolveConfig function call we can get rid of if (isRelativeConfigPath) {...} part.

Please let me know if this helps.

[iiroj]

@antonk52 I don't think we should use require.resolve anymore now that lint-staged is an ESM module.

Anyway, you are right that it was that part that handled the module stuff before!

I actually think I need to extract a bug fix from this branch into a separate PR (the require.resolve).

[iiroj]

@antonk52 FYI: I split a separate PR #1045, and rebased this PR on top of it. So now there's only a single commit with less changes directly related to lilconfig. What do you think?

[antonk52]

Looks great! 👍

One thing I noticed is that you've also added support for ESM, I think this line can be misleading to some as this PR effectively overwrote all default loaders. Perhaps replacing it with something that mentions that lint-staged supports ESM with default export and YAML can be more useful for some people.

[iiroj]

@antonk52 I actually removed that line after I merged this into #1045, mostly for this exact reason.

I was thinking of opening a PR in lilconfig to switch the default async loaders to use import(), but this was simpler for me.

[antonk52]

I would be happy to merge a PR adding ESM support, but this would make lilconfig converge from cosmiconfig feature set. I prefer it to stay as close as possible as a drop in replacement. I think I saw an issue about adding ESM support to cosmiconfig in the next major version. Once this happens I am happy to release the next major version to preserve the compatibility with cosmiconfig.

I've read through #1045 and left a comment. Looks great!

[iiroj]

@antonk52 cool. I think changing the async explorer.load to use import() instead of require would still work exactly the same way for CommonJS modules, but also add support for ESM. Maybe it doesn't need to be a breaking change?

[antonk52]

This will add a difference to how lilconfig and lilconfigSync which should be avoided for as compatibility reasons. Another reason not avoid doing this is that lilconfig v2 supports node v10 much like cosmiconfig v7 does , start using import() would automatically invalidate this support requirement. For these 2 reasons I think adding this feature to lilconfig itself should be avoided for as long as cosmiconfig decides to start supporting ESM. I do like how the API allows the user to redefine how all filetypes will be processed and adding ESM support becomes a pretty straight forward task like what you did in this PR and #1045.

[iiroj]

Makes sense, thanks a lot for the help with this @antonk52.

[antonk52]

Sure thing. Please do ping me if there will be future issues regarding lilconfig