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

types: organize into folder #1024

Merged
merged 14 commits into from
Apr 16, 2021
Merged

types: organize into folder #1024

merged 14 commits into from
Apr 16, 2021

Conversation

ignatiusmb
Copy link
Member

Organize types into its own folder, this will make it easier to develop and reference types in deeply nested files, as well as add more unrelated types if necessary. This PR actions consists of

  • organizing types and publishing the whole directory
  • exposing only public types from types/index.d.ts
  • escaping* relative path hell, reference like normal module

*note: there's still runtime/client/types.d.ts and runtime/server/page/types.d.ts which uses types from the root folder, but this could now be placed in the root types folder as well, it can (in theory) be referenced as types/client and types/server.

No tests or changeset should be needed as this should not change anything

@ignatiusmb ignatiusmb changed the title types: organize into folder and remove double dots types: organize into folder Apr 14, 2021
Copy link
Member

@benmccann benmccann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks very nice. thank you!

@dummdidumm
Copy link
Member

While we're at it: could we also restructure the types internally? Right now they reference each other recursively which signals to me that there's something wrong. Could we change this up so that internal only references public but not the other way round? Everything in public that references internal is public, too, after all.

tried to break them up into each categories

also added helper.d.ts to try avoid circular
dependencies between each files imports
@ignatiusmb
Copy link
Member Author

ignatiusmb commented Apr 15, 2021

I tried my best to break it up even further. Also updated the paths used internally to import it explicitly (from types/foo) instead of from the public (index.d.ts) types directly, disabled it in tsconfig as well so it'll be consistent.

Although internal.d.ts is still a mess, I'm assuming the API aren't quite stable yet so I tried to minimize the changes there.

Edit: There's a lot of SSR and CSR prefixed types in internal.d.ts which I didn't quite know where to put it now (client/server or maybe renderer??) so I'll leave it as is for the time being.

@dummdidumm
Copy link
Member

dummdidumm commented Apr 15, 2021

I think these are good splits. I would suggest to revert the interface name changes though to not introduce a breaking change at the same time when we refactor the types, that should go into a different PR IMO. Edit: Just saw you rename them while reexporting them from index.d.ts, so nevermind 😄

Copy link
Member

@Rich-Harris Rich-Harris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, but can we change types/server to types/endpoint and types/client to types/page? The current division isn't quite accurate, since pages are rendered on the server as well as in the client.

@ignatiusmb
Copy link
Member Author

Done!

I was following how it each types were imported from runtime/client and runtime/server respectively at first, but calling it endpoint and page seems better now.

@Rich-Harris Rich-Harris merged commit 1d76d66 into sveltejs:master Apr 16, 2021
@Rich-Harris
Copy link
Member

excellent, thank you so much!

@ignatiusmb ignatiusmb deleted the kit/types-folder branch April 16, 2021 02:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants