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

Support dynamic libraries LibrariesOptions #325

Open
carmelc opened this issue Jun 3, 2024 · 4 comments
Open

Support dynamic libraries LibrariesOptions #325

carmelc opened this issue Jun 3, 2024 · 4 comments

Comments

@carmelc
Copy link

carmelc commented Jun 3, 2024

Hi,
Currently the ability to define LibrariesOptions is static, you can define inlinedLibraries, importedLibraries and allowedTypesLibraries as static array of strings,

If I want all libraries under an organization (namespace) to be inlined, I would need to maintain a huge list of possible libraries (I can see that if inlinedLibraries is not provided it is defaulted to []), so by default none are inlined
I was thinking about allowing each of these to be a string array (as today) or a method, and if it is a method the logic of isLibraryAllowed would check if the type is a function and if so would invoke the callback with the name of the library in question allowing the user to implement custom logic.
Obviously I can hack it around by using:

const inlinedLibraries = [];
inlinedLibraries.indexOf = (library) => library.startsWith('@namespace') ? 1 : -1;

But this is obviously not recommended :)

This would only work in javascript-based configuration and not in the command line,
We can also consider a regex instead of string equality but I believe allowing the user to implement their logic as code is better.

What do you think? if you are fine with such an approach I don't mind creating the PR for it

@timocov
Copy link
Owner

timocov commented Jun 15, 2024

Hey @carmelc,

This would only work in javascript-based configuration and not in the command line,

That's my worry. For such a simple task it feels like we should provide a way achieving it by using CLI as well instead of forcing everyone to use js-based config file. As an extension later we can introduce a function-based approach if there would be a need for that.

We can also consider a regex instead of string equality

I'd prefer this approach, but I'm not sure if we need to restrict this anyhow i.e. allow to specify namespace-based values only (@company/*) or something like that. Supporting full regex syntax might not be straightforward (considering bash shenanigans with *, / and stuff like that). Do we have a good example of any tool out there that have something similar that we can take a look at/copy?

Feel free to create a PR if you have a solution, we can discuss it there too.

@carmelc
Copy link
Author

carmelc commented Jun 16, 2024

Hi @timocov, thanks for the detailed reply,
About a reference to other libraries with the same need, rollup is a good example:
https://rollupjs.org/configuration-options/#external
As you can see the CLI is a subset of the JS configuration capabilities and not everything supported in the JS config is supported as a CLI argument.
The CLI only supports module ids while the JS config also support RegExp and a function similar to my proposal, this reflects the approach that CLI should not be 1-1 to the JS configuration which is richer and provides all the capabilities of the language,
I agree with your concern about having Regex's as a command line argument so in my opinion it can be a JS config option only.
Let me know if you would like me to implement a js method approach and I will gladly do it

@timocov
Copy link
Owner

timocov commented Jun 16, 2024

rollup is a good example:

Agreed, but the caveat is (maybe it is just my perceptive tho) - I can't imagine running rollup without a config file, while from I've seen running dts-bundle-generator mostly is as simple as dts-bundle-generator -o out.d.ts input.ts (or something along these lines).

The CLI only supports module ids while the JS config also support RegExp and a function similar to my proposal, this reflects the approach that CLI should not be 1-1 to the JS configuration which is richer and provides all the capabilities of the language

I understood that, my worry is that this would be the first time when this difference would take the place, and if we can avoid that without ruining possible DX it would be much better IMO.

@carmelc
Copy link
Author

carmelc commented Jun 16, 2024

Understood, your call, if you prefer not to create such divergence from the current approach then I will not add it, if you decide to allow it, I will create the PR,
I do agree that using CLI for regex is problematic (and might even be a breaking change),
Let me know if you decide to allow JS only capabilities, and I will create the PR,

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

No branches or pull requests

2 participants