fix(emitDts): set rootDir for consistent output structure

[gtm-nayan]

Ref sveltejs/kit#9118 (comment) and microsoft/TypeScript#52889

Marking as draft cause, should it be a hard error or a warning and who is responsible for logging it: emitDts or the caller?

[dummdidumm]

Thanks for figuring that out! This makes sense to me. As for the question: I think emitDts should start returning an object which for now has { warnings: { file: string; message: string; }[] }.

[gtm-nayan]

I made it return an object containing the diagnostics from ts itself, @sveltejs/package should probably filter based on the specific "files outside root" error's code cause otherwise it also contains errors on imports like $app/.. because include is different from .svelte-kit/tsconfig.json.

The test timeout for the plugin seems unrelated (hopefully 😬).

[dummdidumm]

I'm wondering how much slower the emit is becoming through this, AFAICT TS will do some thorough code analyses there, and if there are errors inside Svelte files the diagnostics would need to be mapped etc. It might also close #1835. Should we have this behind an option? Should we leave it out completely for now?

[gtm-nayan]

The problem with rolling our custom warnings is that it doesn't contain information on where the bad import happened. The WriteFileCallback gets a SourceFile I'll see if there's a way to extract this information from there.

Alternatively, I suppose we could set a flag if such files are found inside the callback and only run the diagnostics if it's true?

[gtm-nayan]

As for #1835, I don't think emitDts's diagnostics can be fully relied upon. It already gets a few false positives because the tsconfig passed to it doesn't line up with the generated by SvelteKit.

[dummdidumm]

My idea for generating d.ts files is now that we don't pass the libRoot option anymore, instead only keeping the the generated d.ts files inside of src/lib (or whatever is configured) later on. So I think we should remove the diagnostics feature for now.
If people want to have a check that their generated output is correct they could run tsc on the dist folder's d.ts files afterwards.

[gtm-nayan]

My idea for generating d.ts files is now that we don't pass the libRoot option anymore

Do you mean you want to change it to be that way at some point? Right now it's still there. I wrote my thoughts on not wanting to do that in that other thread, would there be any benefit to it?

If people want to have a check that their generated output is correct they could run tsc on the dist folder's d.ts files afterwards.

Do we trust people to do that or should svelte-package include the check in its scripts?

[dummdidumm]

My idea for generating d.ts files is now that we don't pass the libRoot option anymore

Do you mean you want to change it to be that way at some point? Right now it's still there. I wrote my thoughts on not wanting to do that in that other thread, would there be any benefit to it?

The benefit is that we don't mess around with the tsconfig include/files settings. Input/output settings are no problem because <output>/__package_types_tmp__ is the declaration dir and we flatten the directory structure afterwards (relative(tsconfigdir, input)). It's possible slower though, that is right. Maybe we can do this later if more problems arise.

New proposal: Don't return any diagnostics. Just emit a one-time warning when we detect that some file would be written out that is outside the root dir.

If people want to have a check that their generated output is correct they could run tsc on the dist folder's d.ts files afterwards.

Do we trust people to do that or should svelte-package include the check in its scripts?

We trust people to do that if they want to. It's finicky to build it into svelte-package, I wouldn't want to do that right now - the related issue is somewhat of an edge case to me.

[gtm-nayan]

Okay, makes sense.

Although if we consider the project root to be the rootDir, we can't really tell the files apart to emit an warning.

To clarify, should I keep it scoped to the input for now and just remove the diagnostics or change the PR to the other method?

[dummdidumm]

I'm switching my mind constantly 😅 I now think we should just set rootDir to config.libRoot (if it's undefined then rootDir is, too) and on the SvelteKit side we can decide to set it to "." to say "relative to the tsconfig"

[gtm-nayan]

Yeah that's what I meant, should this stay in place for now? https://github.com/sveltejs/language-tools/pull/1886/files#diff-783d276c3f0448d53760c169c6843c026704cd3329bcabbbd46c6234cdcef69bR47

[dummdidumm]

Yeah that makes sense!