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

Private types are exported #153

Closed
RunDevelopment opened this issue Apr 8, 2021 · 9 comments · Fixed by #155
Closed

Private types are exported #153

RunDevelopment opened this issue Apr 8, 2021 · 9 comments · Fixed by #155
Assignees
Milestone

Comments

@RunDevelopment
Copy link

Bug report
This bundler exports private types. ("Private" as in "not exported". This has nothing to do with private fields.)

This is a problem because this means that you can no longer split one complex type into many simpler types.

Input code

This is a very simple example. In reality, B is reused across different exported types but kept private because it doesn't make sense on its own. Users of my library shouldn't be able to refer to B, hence it is not exported.

export type A = string | B;
type B = number;

Expected output

// Generated by dts-bundle-generator v5.8.0

export declare type A = string | B;
declare type B = number;

export {};

Actual output

// Generated by dts-bundle-generator v5.8.0

export declare type A = string | B;
export declare type B = number;

export {};

Additional context
CLI: npx dts-bundle-generator -o my.d.ts src/foo.ts

@timocov
Copy link
Owner

timocov commented Apr 9, 2021

I think this is one of features why dts-bundle-generator exists 🙂 So you don't need to specify all exported types and you can specify the only several of them you'd like to specify and all other will be automatically exported. For instance here https://github.com/tradingview/lightweight-charts/blob/master/src/index.ts we don't export all types but just some of them which we have to export directly (like enums because of generating JS code for them) and it works perfectly.

On the other hand, this is the one of the oldest feature in dts-bundle-generator, which was added a long time before putting export {}; was introduced (just note that without that export export {} you can import any type/feature declared in the d.ts file so thus export has no effect).

Can you please provide a use case when it might be useful? I do understand cases when this is true for classes/functions/enums/any other stuff which affects JS code (and the tool handles this already), but why you want to get the same for interfaces/types? Your users won't be able to import these types and declare variables/arguments with there parameters (I used to use Spotify NodeJS API so time ago and they don't export some of auto-generated types and this was awful experience to use that API, because instead of just importing a type you have to write something like type['key']['another-key']['one-more-key']['and-so-on']).

But I don't mind to add a new option to disable this behaviour if there is a real use case for that.

@RunDevelopment
Copy link
Author

Your users won't be able to import these types

That's the idea. I don't want them to be able to import them.

I sometimes have to write quite long and complex types. To make them more readable and to avoid code duplication, I split those complex types up into smaller helper types. These helper types should not be exported because they are only used to implement the complex type. If the implementation of the complex type changes, I might change/remove the helper types. If people are able to import these helper types, I cannot change/remove them without breaking their code.

Put simply, if all types are exported, I have no control over the API of my library.

Example:

type NodeIdent = { type: Node["type"] };
type NoParentArray<T> = { [K in keyof T]: NoParent<T[K]> };
type NoParentNode<T extends NodeIdent> = { [K in keyof NoParentNodePick<T>]: NoParent<NoParentNodePick<T>[K]> };
type NoParentNodePick<T extends NodeIdent> = Pick<T, Exclude<keyof T, "parent">>;
/**
 * A view of an AST node that hides the `parent` property.
 */
export type NoParent<T> = T extends NodeIdent ? NoParentNode<T> : T extends unknown[] ? NoParentArray<T> : T;

The NoParent type takes an AST node type had hides the parent properly of the AST node type and all of its descendants. All non-exported types are just used to implement the NoParent type and may change from version to version. Having them be publicly accessible is not good.

(Sometimes splitting a recusive type into multiple types is sometimes a necessity due to TypeScript's recursion limitations.)


I also want to add that I find it quite surprising this bundle auto-exports types. Other bundlers (like dts-bundle) do not do this. Is this behavior documented?

@timocov
Copy link
Owner

timocov commented Apr 10, 2021

I sometimes have to write quite long and complex types. To make them more readable and to avoid code duplication, I split those complex types up into smaller helper types. These helper types should not be exported because they are only used to implement the complex type. If the implementation of the complex type changes, I might change/remove the helper types. If people are able to import these helper types, I cannot change/remove them without breaking their code.

Totally makes sense. Thanks for the clarification! I think we can add an option for that, something like exportDirectlyUnexportedTypes or so (maybe you can provide a shorter & better readable name, like exportDependantTypes), I think it should be used somewhere here.

I also want to add that I find it quite surprising this bundle auto-exports types. Other bundlers (like dts-bundle) do not do this. Is this behavior documented?

Are you about "mark all interfaces/types exported"? If so, then I'm not sure about "documented", but it's intended for sure. I mean, back in time "exporting all types in bundles" was one of the main reasons to create this tool.

@timocov
Copy link
Owner

timocov commented Apr 10, 2021

@RunDevelopment would you like to propose a PR with changes?

@RunDevelopment
Copy link
Author

Are you about "mark all interfaces/types exported"?

Yes.

If so, then I'm not sure about "documented", but it's intended for sure. I mean, back in time "exporting all types in bundles" was one of the main reasons to create this tool.

Then this should be documented for sure. You might want to tell people about your USP ;)

maybe you can provide a shorter & better readable name

That's hard. Let me start by saying that exportDependantTypes is wrong. dependant means that the private types depend on other (exported) types. But it's the other way around, the exported types need the private types.

How about exportReferencedTypes? These types are exported because an exported type references them after all.

would you like to propose a PR with changes?

I'm sorry but no.

@timocov
Copy link
Owner

timocov commented Apr 10, 2021

Then this should be documented for sure. You might want to tell people about your USP ;)

I think we can add a note for the brand new option which describes this behavior enabled by default.

How about exportReferencedTypes? These types are exported because an exported type references them after all.

Agree. I think exportReferencedTypes is much better for this case.

@RunDevelopment
Copy link
Author

Thank you @timocov!

@timocov
Copy link
Owner

timocov commented Apr 13, 2021

The new version 5.9 has been published with the fix.

@RunDevelopment
Copy link
Author

I just tested it out and it worked beautifully as expected.

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