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

feature request: more possible config file names #182

Closed
Zamiell opened this issue Aug 1, 2023 · 23 comments
Closed

feature request: more possible config file names #182

Zamiell opened this issue Aug 1, 2023 · 23 comments
Labels
feature request Feature request

Comments

@Zamiell
Copy link
Contributor

Zamiell commented Aug 1, 2023

Currently, knip supports these configuration names:

  • knip.json
  • knip.jsonc
  • .knip.json
  • .knip.jsonc
  • knip.ts
  • knip.js
  • package.json#knip

However, I think this should be expanded. First of all, it is common to name .ts and .js files with an extension according to whether or not the file is CommonJS / ESM. Meaning that knip should support:

  • knip.mts
  • knip.cts
  • knip.mjs
  • knip.cjs

Additionally, I think that Knip should follow the style of the existing tools in the ecosystem. Most notably, ESLint has a config file of eslint.config.js and Prettier has a config file of prettier.config.mjs and Jest has a config file of jest.config.mjs. So, I think Knip should support:

  • knip.config.ts
  • knip.config.mts
  • knip.config.cts
  • knip.conifg.js
  • knip.config.mjs
  • knip.config.cjs

Furthermore, the docs should actively recommend that people use this config file naming format so that we match the rest of the ecosystem.

Additionally, note that this is more than a semantic or stylistic issue. TypeScript throws errors for a knip.ts when it is inside of a CommonJS project. Renaming the file to knip.mts fixes the problem, but then knip no longer picks up the file. So this is a blocker. (This can be worked around with the --config flag.)

@Zamiell Zamiell added the feature request Feature request label Aug 1, 2023
@Zamiell
Copy link
Contributor Author

Zamiell commented Aug 1, 2023

I can try a PR for this if needed.

@webpro
Copy link
Collaborator

webpro commented Aug 2, 2023

You can use --config [file] (or -c) for this. Or does any of the file formats cause issues with that?

@Zamiell
Copy link
Contributor Author

Zamiell commented Aug 2, 2023

You can use --config [file] (or -c) for this.

You're right, so this isn't a blocking issue, but rather a nice-to-have convivence for end-users.

@webpro
Copy link
Collaborator

webpro commented Aug 2, 2023

Just wondering, is there any tool out there that tries to read all of those variations?

@Zamiell
Copy link
Contributor Author

Zamiell commented Aug 2, 2023

And probably more? Not sure.

Note that ESLint explicitly does not, but many users are mad/frustrated about this because it goes against the rest of the ecosystem and Node.js itself!

@webpro
Copy link
Collaborator

webpro commented Aug 3, 2023

Thanks. Not sure whether all of them offer similar to what you request here though ;)

Anyway, I'm in camp "explicit is usually better", which is also one of the ideas in ESM (explicit extension). The mess with migrating from CJS to ESM is unfortunate, but I'm not sure whether tooling should also maintain the same state of hybrid/complexity.

Over time the list of conventions keeps growing and growing. It would be great if the ecosystem adopted a convention such as a config folder. But that would probably only increase the problem for the same reason.

Since Knip offers a solution for cases that need a different solution I'm going to close this issue.

@what1s1ove
Copy link
Contributor

what1s1ove commented Dec 17, 2023

Thanks. Not sure whether all of them offer similar to what you request here though ;)

Anyway, I'm in camp "explicit is usually better", which is also one of the ideas in ESM (explicit extension). The mess with migrating from CJS to ESM is unfortunate, but I'm not sure whether tooling should also maintain the same state of hybrid/complexity.

Over time the list of conventions keeps growing and growing. It would be great if the ecosystem adopted a convention such as a config folder. But that would probably only increase the problem for the same reason.

Since Knip offers a solution for cases that need a different solution I'm going to close this issue.

I understand you, but I still think it would be cool to have knip.config.js as a possible config file name, as most other configuration files have this prefix or plan to have it (usually with a migration to ESM). It seems to me that it's like a standard for JS tools now.

image

@webpro
Copy link
Collaborator

webpro commented Dec 17, 2023

Thanks for the input. What does red strike-through mean?

Just checked one sample: turns out release-please has only release-please-config.json (note the dashes) as a default config file it looks at.

@webpro
Copy link
Collaborator

webpro commented Dec 17, 2023

Just had someone else expecting knip.config.js to work as well, so I'm leaning towards adding it to the list (in upcoming v4 release). I don't want to be annoying here (I know I am), it's just all so fragmented.

@what1s1ove
Copy link
Contributor

what1s1ove commented Dec 17, 2023

Thanks for the input. What does red strike-through mean?

Just checked one sample: turns out release-please has only release-please-config.json (note the dashes) as a default config file it looks at.

I simply used the native browser search (Command + F) to find the word 'config' in one of my repositories (highlighted in yellow).

I crossed out in red those configs that have 'config' in their name but are not actually configs (or like knip.config.js which currently doesn't work by default (only through additional flags)). The ones highlighted in red are what you don't need to look at. Just focus on the yellow color.

@Zamiell
Copy link
Contributor Author

Zamiell commented Dec 17, 2023

@webpro eslint/eslint#17863 was opened 2 days ago.
Given that ESLint has changed its stance on this, and that Prettier already supports this format, maybe Knip should reconsider?

@Zamiell
Copy link
Contributor Author

Zamiell commented Dec 17, 2023

It would be great if the ecosystem adopted a convention such as a config folder. But that would probably only increase the problem for the same reason.

I've seen this link floating around in some of the places I frequent: https://dot-config.github.io/
If you agree, then it also might be worth supporting ".config/knip.json" out of the box in Knip.
For more interesting discussion, see:

@webpro
Copy link
Collaborator

webpro commented Dec 17, 2023

@webpro eslint/eslint#17863 was opened 2 days ago. Given that ESLint has changed its stance on this, and that Prettier already supports this format, maybe Knip should reconsider?

That's a different discussion. The new ESLint flat config file name is eslint.config.js, this discussion is about also supporting the .mjs and .cjs extension for flat configs.

It would be great if the ecosystem adopted a convention such as a config folder. But that would probably only increase the problem for the same reason.

I've seen this link floating around in some of the places I frequent: https://dot-config.github.io/ If you agree, then it also might be worth supporting ".config/knip.json" out of the box in Knip. For more interesting discussion, see:

This doesn't make it better? Unless the ecosystem reaches consensus about the folder name: /.config or /config

@Zamiell
Copy link
Contributor Author

Zamiell commented Dec 17, 2023

I think the idea of the authors is that the ecosystem should specifically converge around /.config.

image

@Zamiell
Copy link
Contributor Author

Zamiell commented Dec 17, 2023

That's a different discussion. The new ESLint flat config file name is eslint.config.js, this discussion is about also supporting the .mjs and .cjs extension for flat configs.

Sorry, you're right, I didn't re-read this thread carefully enough.

@webpro
Copy link
Collaborator

webpro commented Dec 17, 2023

I think the idea of the authors is that the ecosystem should specifically converge around /.config.

Others think it should be an un-dotted config, e.g. https://github.com/natemoo-re/proload (but it's really just one example).

Interesting discussion still, but as much as I love to have a common default everyone & everything actually respects, I'm not very confident this will ever happen.

@Zamiell
Copy link
Contributor Author

Zamiell commented Dec 17, 2023

Others think it should be an un-dotted config, e.g. https://github.com/natemoo-re/proload (but it's really just one example).

Interesting. I opened up an issue over there, I am curious to see what they think about the recent push for ".config" instead of "config".

Interesting discussion still, but as much as I love to have a common default everyone & everything actually respects, I'm not very confident this will ever happen.

Well, we got most of the ecosystem to converge around Prettier, so you never know. =p

@webpro
Copy link
Collaborator

webpro commented Dec 17, 2023

One of the reasons I think it's hard to reach consensus is that the .config idea mainly originates from the XDG spec, right? But the frontend world is not that Linux/dotfiles oriented (there's also Windows in that area), and config files are often even authored in JS/TS (in contrast to INI/TOML/what have you). Hiding dotfiles, which is often the default when listing directory items (e.g. when using ls or in macOS Finder), might be too big of a change for frontend folks.

Either way, as mentioned, I'm leaning towards adding the knip.config.* file convention to the list.

@webpro
Copy link
Collaborator

webpro commented Dec 19, 2023

🚀 This issue has been resolved in v4.0.0-canary.3. See Release 4.0.0-canary.3 for release notes.

@webpro
Copy link
Collaborator

webpro commented Dec 19, 2023

It's not a breaking change, but I'd like to add it to the v4 changelog. You can try v4 canary today including this change with e.g. npm install knip@canary. Documented at https://knip.dev/v4/overview/configuration#location.

@Zamiell
Copy link
Contributor Author

Zamiell commented Dec 19, 2023

What about adding ".config.json" and ".config.jsonc"? Was that intentionally skipped?

@webpro
Copy link
Collaborator

webpro commented Dec 19, 2023

What about adding ".config.json" and ".config.jsonc"? Was that intentionally skipped?

No, it was not even requested.

@Zamiell
Copy link
Contributor Author

Zamiell commented Dec 19, 2023

Ah, well that might be worth adding as well. CSpell already supports "cspell.config.json" and "cspell.config.jsonc", for example.

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

No branches or pull requests

3 participants