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

Incorrectly leaves import in place #34

Open
benmccann opened this issue Aug 17, 2023 · 5 comments
Open

Incorrectly leaves import in place #34

benmccann opened this issue Aug 17, 2023 · 5 comments

Comments

@benmccann
Copy link
Contributor

sveltejs/kit#9605 includes a line in navigation.js:

@type {(callback: (navigation: import('@sveltejs/kit').OnNavigate) => MaybePromise<(() => void) | void>) => void}

I believe this is incorrect. However, dts-buddy is still able to generate some output:

export const onNavigate: (callback: (navigation: import('@sveltejs/kit').OnNavigate) => MaybePromise<void | (() => void)>) => void;

I think the line in navigation.js should be updated to include import('types'):

@type {(callback: (navigation: import('@sveltejs/kit').OnNavigate) => import('types').MaybePromise<(() => void) | void>) => void}

If I do so then dts-buddy then generates:

export const onNavigate: (callback: (navigation: import('@sveltejs/kit').OnNavigate) => import('types').MaybePromise<(() => void) | void>) => void;
...
type MaybePromise<T> = T | Promise<T>;

It seems correct that dts-buddy then generates a type declaration for MaybePromise, but incorrect that it leaves the import('types') in place.

@Rich-Harris
Copy link
Owner

Is this still happening? There were no typechecking issues on the PR after upgrading to the latest dts-buddy, does that mean this can be closed?

@benmccann
Copy link
Contributor Author

I expect it is still happening if you revert the commit @s3812497 added to the PR that he was calling a workaround: sveltejs/kit@c68d625. I'm not sure exactly why that fixed it or if we want to say that's how you have to use it rather than calling it a workaround

@eltigerchino
Copy link

eltigerchino commented Aug 31, 2023

For some reason it couldn't resolve the 'types' path that was aliased in the package.json file. The relative path and exported @sveltejs/kit path seemed to be recognised instead.

@Rich-Harris
Copy link
Owner

FYI I think this is fixed in the latest dts-buddy, though I haven't had a chance to check just yet

@Rich-Harris
Copy link
Owner

sveltejs/kit#11013

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

No branches or pull requests

3 participants