fix: don't remove namespaced imports if they are external

[paoloricciuti]

I've added a declaration file for all-as-external in the test because otherwise the fix was not really visible (i didn't use external or it would've effected the other tests)

[changeset-bot]

🦋 Changeset detected

Latest commit: 9428527

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
dts-buddy	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Rich-Harris]

I approved this before running the tests... it's failing because on older versions of TypeScript this gets generated:

declare module 'import-external-all-as' {
	import * as v from 'all-as-external';
	/// <reference path="external.d.ts" />
	export const Schema: v.BaseType<string>;
	export type Test = v.Infer<typeof Schema>;

	export {};
}

//# sourceMappingURL=index.d.ts.map

Is that harmless to leave in or does it need to be stripped out?

[paoloricciuti]

Mmm i suppose is harmless and i think it's only generated in this case because i'm using the external.d.ts to show the result...let me check quickly

[paoloricciuti]

So testing with an actual library the path is not generated...however someone might actually do this? In theory the reference comment should be at the top of the file but i don't know how TS will behave if it finds one in a weird place and i don't know if we can just remove it without causing problems...let me test a bit

[paoloricciuti]

I think the right move is to move them at the top of the .d.ts they serve to tell that that declaration file is referencing the other file and it's indeed doing it. We can access the referenced file from module.ast.referencedFiles but:

1. the position is super weird because they only give you the position of the file name (not the whole /// <reference position...so if there's extra whitespace for some reason it could be annoying
2. i was starting to just remove them but i realised that we are actually moving them...not sure how this would affect sourcemaps

WDYT?

[paoloricciuti]

Even tho reading at the reason they removed them i would say we can go the typescript route and just remove them 😄

https://devblogs.microsoft.com/typescript/announcing-typescript-5-5/#simplified-reference-directive-declaration-emit

[paoloricciuti]

Mmm on a second thought: I think if we do encounter such behaviour we should probably bundle that reference too. For libraries it's fine because if they are part of your api you should include them as a dependency so even the host project it should resolve that name. But if you do the same degenerate thing i did here (create a .d.ts that declare a module) for some reason then that module will not be available in the host project if we don't bundle it.

This kinda opened a can of worms 🪱