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

Option to specify configuration in the manifest #72

Open
steelbrain opened this issue Nov 29, 2024 · 6 comments
Open

Option to specify configuration in the manifest #72

steelbrain opened this issue Nov 29, 2024 · 6 comments
Labels
feature New feature or request

Comments

@steelbrain
Copy link

Hello!

Would you be interested at all in adding an option to specify configuration, not from the CLI, but within package.json? This would be similar to eslintConfig, prettier, ava etc.

So then a manifest file could look something like this

{
  "name": "hello-world",
  "removeUnused": {
    "project": "client.tsconfig.json",
    "skip": ["a", "b"]
  }
}

Then users would be able to invoke ts-remove-unused without having to use the programmatic API or having really long commands

@kazushisan
Copy link
Contributor

kazushisan commented Nov 30, 2024

Thanks for the suggestion! 🙌 Here are my thoughts on introducing config files:

  • Adding config files would likely mean supporting various formats (e.g., .js in addition to .json) for a better editing experience and to support complex configurations. However, implementing this correctly involves significant effort — such as transpiling/bundling beforehand to handle import statements in config files, and correctly switching the load logic for CJS/ESM compatibility. For a new project like this, I feel a programmatic API (similar to esbuild) is a more straightforward and effective approach.
  • Config files can introduce implicitness, while an API is more explicit, making it clearer which options are being used at runtime. Config files also assume a certain project structure (with a config file at the root), which might not align well with this tool since running ts-remove-unused multiple times with multiple tsconfig files (like server and front-end) within the same project is much more likely than say prettier, which would be unlikely to have multiple configs necessary within the repo.

For these reasons, I’m leaning toward not introducing config files. That said, if I’ve overlooked any benefits that are unique to config files, please let me know! I’d also be happy to hear any ideas on improving the API to better address your needs.

@steelbrain
Copy link
Author

If you have strong preferences against configuration files, I would completely understand and drop the pursuit. I do agree with the points you've mentioned. ESLint, for example, has been famously hard to figure out active runtime configuration for, despite having a --print-config option.

What I'm asking for is something extremely bare bones. In my proposal:

Do:

  • Support specifying configuration in package.json under a property
  • Require a cli arg to be specified to use the config (--project maybe?)
  • Merge the command line args if specified into the config

Don't:

  • Create a .ts-remove-unused.{js{on},yml}
  • Read the config file by default

If implemented in such a way, you don't need to transpile/bundle anything, as the package manifest file is plain json, and there is no implicit configs as the cli specifically needs the path where the config is picked up from.

Please let me know if this makes sense, or if it doesn't, I'll try to explain further. I'd like to reiterate that if you have strong feelings against config files, that's also a world we can live in. Thanks!

@kazushisan
Copy link
Contributor

kazushisan commented Dec 1, 2024

I thought your suggestion covered all types of config files, sorry for the misunderstanding. I think its worth giving a shot! If you’re interested in implementing, please let me know

@steelbrain
Copy link
Author

I'd love to work on this and put up a PR!

@steelbrain
Copy link
Author

steelbrain commented Dec 3, 2024

Which branch/PR do you think I should use as a base? I see some fun changes in #74

@kazushisan
Copy link
Contributor

kazushisan commented Dec 3, 2024

Great! I think #74 is a better choice if you're not in a real hurry 👍

@kazushisan kazushisan added the feature New feature or request label Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants