fix: add ambient types to published files

[ignatiusmb]

Publish ambient types added by #917. Sorry, I didn't get to catch this before it was merged

• Newly added ambient types isn't detected because the file doesn't exist
• New projects created with TypeScript enabled will also complain that the modules doesn't exist
• Plain JS projects still has autocompletion because it still has a hardcoded $app/ in its jsconfig.json

[dummdidumm]

Thanks for catching this! Could you adjust the jsconfig as well?

[ignatiusmb]

Done! I suppose one changeset is enough? It's a different package, I'll update it then

[benmccann]

I'm still having a really hard time understanding ambient.d.ts vs types.d.ts. I don't quite understand why there are two files or how we decide what goes in each one

The explanation on the original PR https://github.com/sveltejs/kit/pull/917/files/51a1212c29801a473304e06a3194a5d8f206f9ba#r611350994 was that ambient.d.ts must contain the top-level imports, but I don't see any imports in that file, so I'm not understanding yet

I also think the name "ambient" is a confusing way to name the file because as far as I understand the types in types.d.ts are also ambient - though I may be wrong on this point, so feel free to correct me

[ignatiusmb]

It's probably just how types are organized within a certain codebase, there's really no right or wrong though as long as it works. But, having ambient modules certainly helps make it cleaner.

The name "ambient" simply means it is without implementation. It's similar to how compiled TypeScript will generate a .js and .d.ts files with declare keywords everywhere in the declaration file, meaning "assume there will be a variable named foo with type string already defined elsewhere (in this case, compiled .js file). We can also declare these manually (when not using tsc) in one .d.ts where we specify each modules (e.g. $app/env or $app/stores) and have it available everywhere in the project, which is what #917 did.

ambient.d.ts must contain the top-level imports, but I don't see any imports in that file

I think it's the other way around, it should not have any top-level imports. Perhaps it could be considered as global types in some ways where we usually declare module "module-without-declaration-types" to silence TypeScript in our global.d.ts file.

[benmccann]

The name "ambient" simply means it is without implementation

That's what I thought. I believe the types in types.d.ts are also without implementation and are thus ambient. That makes it confusing to me to have a file called ambient.d.ts that contains only some of our ambient types. Maybe we could call it modules.d.ts or something more reflective of what the separation between the files actually is?

What's the issue with putting all the types in a single types.d.ts?

[ignatiusmb]

The types in types.d.ts are implemented with other files import-ing from it.

What's the issue with putting all the types in a single types.d.ts?

I'm not 100% certain on this, but declare module "$app/*" { ... } will not work when there's a top-level import/export as well. Even if it works, having them in a different file will also be easier to maintain and makes the separation between the types clearer. (Edit: Just double-checked and confirmed, it does not works)

Combining them in one types.d.ts would probably clutter the file too much, it's just like how we separate our utility functions into a separate file out of the logic file when it gets too big.

That makes it confusing to me to have a file called ambient.d.ts that contains only some of our ambient types. Maybe we could call it modules.d.ts or something more reflective of what the separation between the files actually is?

Why do you think a file called ambient.d.ts that contains only some of our ambient types? Isn't the point of naming it ambient.d.ts is to show that it only contains ambient types?

I think naming it modules.d.ts would be more confusing because according to ts(1208), "Add an import, export, or an empty 'export {}' statement to make it a module". Also, to quote a bit from TS docs about referencing a module

The compiler will try to find a .ts, .tsx, and then a .d.ts with the appropriate path. If a specific file could not be found, then the compiler will look for an ambient module declaration.

Which means any file with an import/export a module file, including plain .ts. On the other hand, ambient modules are typically defined in .d.ts files.

---

Is there anything else besides the naming and multiple type files? We could certainly put everything in one types.d.ts including types.internal and just publish that. We could also separate it even more into a types/ folder with an index.d.ts, publish the whole folder and point to the index.d.ts in it as the entry point.

[benmccann]

I'm fine having two files and having them where they live. It's just the name that bothers me. Ambient is meaningless in my mind because every .d.ts file I've seen contains ambient types. I think it'd be helpful if you know from the name what might be contained within the file. E.g. if I see a file called server.ts I know roughly what will be in it. But with a file called ambient.d.ts I have no idea what to expect. It might as well be called types2.d.ts or anything else.

The TypeScript docs call the construct "ambient modules": https://www.typescriptlang.org/docs/handbook/modules.html#ambient-modules. Would ambient-modules.d.ts work for you?

[dummdidumm]

I did ambient.types.d.ts because they are then grouped at the bottom. I'm okay with ambient-modules. I'm against dropping ambient from the name because that's just the nomenclature.

[benmccann]

thanks for the rename

[benmccann]

hmm. looks like there's a merge conflict now

[dummdidumm]

I generally agree that the types need some more cleanup. I like the idea of adding a types folder with index.d.ts. This would also help resolving some of the circular imports between the d.ts files. Should be a separate PR though.