svelte icon indicating copy to clipboard operation
svelte copied to clipboard

fix: export types needed for defining custom stores

Open vrde opened this issue 2 years ago • 6 comments

I wrote a custom store to have a more flexible derived store. The function signatures looks like:

export function derivedWritable<S extends Stores, T>(
  stores: S,
  fn: (values: StoresValues<S>, set: (value: T) => void) => Unsubscriber | void,
  initial_value?: T
): Writable<T> { ... }

Both Stores and StoresValues are not exported in svelte/store. I had to copy paste the type definition from this file and duplicate code. Unless I'm using TypeScript wrong (I'm still quite new to it), I think these types plus few others should be exported.

Before submitting the PR, please make sure you do the following

  • [ ] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • [x] Prefix your PR title with [feat], [fix], [chore], or [docs].
  • [x] This message body should clearly illustrate what problems it solves.
  • [ ] Ideally, include a test that fails without this PR but passes with it.

Tests

  • [ ] Run the tests with npm test and lint the project with npm run lint

vrde avatar Mar 10 '22 23:03 vrde

Related, I want to use the types SpringOpts and SpringUpdateOpts from svelte/motion but they're not exported.

I'm using this workaround for the spring module to avoid copy/pasting, but this strategy does not work for all of the types exported in this PR:

type SpringOpts = Exclude<Parameters<typeof spring>[1], undefined>;
type SpringUpdateOpts = Exclude<Parameters<ReturnType<typeof spring>['update']>[1], undefined>;

My apologies if this is the wrong place for this. In my case it's a trivial issue.

ryanatkn avatar Mar 20 '22 18:03 ryanatkn

:wave: Any updates? The PR is ready, all tests pass, is there anything else left? Thanks!

vrde avatar Apr 14 '22 10:04 vrde

@baseballyama @bluwy: would it make sense to update this PR to export also the types mentioned by @ryanatkn? Wondering if there are more types that should be exported :thinking:.

PS: sorry for the direct mention, just wanted to move this forward if possible :heart:

vrde avatar Aug 17 '22 07:08 vrde

Just came to make this same PR. I keep having to manually add export directives each time I update the package.

jrmoynihan avatar Sep 12 '22 21:09 jrmoynihan

AFAIK @Conduitry had reservations of exposing the Invalidator type because strictly speaking that's internal API - though since it's available in other public types, that's kind of a lost cause.

dummdidumm avatar Feb 28 '23 17:02 dummdidumm

AFAIK @Conduitry had reservations of exposing the Invalidator type because strictly speaking that's internal API - though since it's available in other public types, that's kind of a lost cause.

Not sure why you say it's a lost cause. You have devs that are asking for these types to be exported. That's a clear need. I'm not sure why this is not part of the discussion.

vrde avatar Jun 05 '23 15:06 vrde

Closing this PR because in the meantime I moved to react 😂

vrde avatar Jul 02 '24 20:07 vrde