Referenced types and renamings

[Atrue]

It’s kind of duplicate of #153 but I’d like to bring it up again.

Since there is a new collision-resolver feature, some types\enums\variables have a new name in a final file (like export type Type$1 = string). The main idea is to have a main TS logic works properly inside a d.ts file and to export only the result of this logic. So it looks like these types should be hidden by default.

For example, it may confuse someone if some autocomplete tools in an IDE suggest something like do you want to import Type$1 from ‘lib’?

I know about the exportReferencedTypes flag, but according to the above I think there are no much sense of this feature and maybe it’s better to remove it at all (make it false) or to disable it by default

[timocov]

I thought we have a logic not to export items if their "public" name if different from internal 🤔 do you have a repro when it doesn't work?

[Atrue]

Sure. You can check this commit https://github.com/Atrue/dts-bundle-generator/commit/a94d3b381a3bc2667c986f124dbd82a0330cd2a3

The output file is:

...
export interface Hidden {
  type?: typeof hidden;
}
...
export interface Hidden$1 {
  type?: typeof hidden$1;
}

The exportReferencedTypes: false flag works as expected, so with this flag the result is:

interface Hidden {
  type?: typeof hidden;
}
...
interface Hidden$1 {
  type?: typeof hidden$1;
}

But the idea is to have the second variant as default as I don't think there is a reason to export something like Hidden$1

[timocov]

Released in https://github.com/timocov/dts-bundle-generator/releases/tag/v9.2.3 just now.

[timocov]

I hope this is what you'd expect @Atrue ? It feels like this behavior at least fixes the wrong output in case of exportReferencedTypes option enabled. We can discuss possible default value change for exportReferencedTypes in a separate ticket tho (as it would be a breaking change), but it feels like it is helpful in most of the cases (e.g. you can export just a function and all referenced/used types are exported automatically for you - but I agree it might be unexpected that some of the types are exported but not the others that have name conflict with global types...).

[Atrue]

but I agree it might be unexpected that some of the types are exported but not the others that have name conflict with global types

Not sure It’s what’s I expected. https://github.com/Atrue/dts-bundle-generator/commit/4ca6b593ed30863616d798bdd66d760264ee48a2 The result is really unexpected

I’ll try to describe the problem here, as we are here in the same context:

• Exporting referenced types was an interesting idea, but let’s see from the other view. This library is a bundle tool. So the expectation of the bundle is to give a result that is compatible with the given code. And if a developer decided not to export something, there might be a reason for that (of course there might be his fault). But here we have an option to disable that - exportReferencedTypes, so let’s say it’s a feature.

• 9.2.3 version filters out some referenced result to export. It means the default feature exportReferencedTypes is not determenistic (rought, but I mean here the logic of name collision of the all given code). So a developer should made a choice here:

  a) (exportReferencedTypes = true) check the output of each bundle run to detect missing referenced types because of collisions

  b) (exportReferencedTypes = false) just write regular code and export types he wants in the index file (like he did it without the bundle, 100% compatible)

• deleting the exportReferencedTypes feature or disabling it by default is a breaking change, I agree. But you can help developers decide what to do in the next releases by making this option required, for example. So next time a developer updated the library and run the bundle without passing this option, you can show an error with some suggestions and clarifications for the option.

• This problem became visible because of the latest name collision feature. So it looks like exportReferencedTypes feature can’t exist with the collision feature. And of course collision feature is much more helpful here.

[timocov]

@Atrue thanks for the detailed explanation!

I'm tempted to agree with you. I agree that having enabled by default feature that provides unpredictable output is not the best DX and it should be disabled. But on addition to that, I see that just having a feature that is not experimental and provides unpredictable output isn't good either.

So what I'm thinking about right now is the following:

• in the next patch release I'll add warning every time this feature is enabled (or not disabled 😄) AND a type cannot be exported because it was renamed due the name collision. Something like "Filename: a type Name cannot be exported because it was renamed due the name collision. Consider export it explicitly"
• in the next major release (I hope next after the patch) this feature will be disabled by default
• also in the next major release, if this feature is explicitly enabled but the type cannot be exported due the name collision the tool will fail. The warning will still be there, but rephrased to something like "... Either export it explicitly or disable this feature"

It feels like when a user's intention was to enable a feature then this feature should be stricter and fail the build if user's desire cannot be satisfied.

What do you think?

[Atrue]

Sounds great. Thanks!

[timocov]

I just created #288 and #289 for tracking. Thanks for the feedback @Atrue, this was super helpful!