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

Export Types #276

Closed
wuifdesign opened this issue Dec 10, 2019 · 11 comments
Closed

Export Types #276

wuifdesign opened this issue Dec 10, 2019 · 11 comments

Comments

@wuifdesign
Copy link

Would it be possible to export the types used in this project. i want to extend some functionallity and need to copy&paste many of them to my code, so i can access them.

i.e: CaseReducerActions, RestrictCaseReducerDefinitionsToMatchReducerAndPrepare, ...

@phryneas
Copy link
Member

phryneas commented Dec 10, 2019

Hi @wuifdesign, some of these types are deliberately not exported, as they are really just useful internally and we might replace them with simpler types as we find better solutions.
As soon as we export an internal type, essentially we have to support it forever and can never change it to not break things for people.
Especially RestrictCaseReducerDefinitionsToMatchReducerAndPrepare is a type that I am expecting to remove soon using the technique displayed in this playground.

Exporting CaseReducerActions would also require us to export SliceCaseReducerDefinitions, which relies on PayloadActions and some of these might also become obsolete using the same technique.

Could you please explain with a few code snippets what you are trying to do?
Maybe I can help you around your problems with only our exported types - and maybe that'll even lead to something we can add to our TypeScript documentation afterwards :)

PS: using those types should not be necessary for you and should even be avoided, as overspecifying your types instead of using inteference will erase useful type information and make your types overall worse in the end.

@wuifdesign
Copy link
Author

wuifdesign commented Dec 11, 2019

i wont to extend the createSlice a bit. something like that (only as example, i want to add thunks or sagas or something else):

export function createAdvancedSlice<State, CaseReducers extends SliceCaseReducerDefinitions<State, any>>(
  options: CreateSliceOptions<State, CaseReducers> & RestrictCaseReducerDefinitionsToMatchReducerAndPrepare<State, CaseReducers>,
  advancedOptions: any = {},
) {
  const { test } = advancedOptions;
  const slice: Slice<State, CaseReducers> = createSlice(options);

  return {
    ...slice,
    test,
  };
}

and to keep typehinting for options i need this types.

@phryneas
Copy link
Member

phryneas commented Dec 11, 2019

Hm. I understand you use case.

Of course, we could do something like exporting those types under a different name like __WARNING_UNSUPPORTED_SliceCaseReducerDefinitions, but in the end that would not change the underlying problem: we will change these types (definitely their Generics Signature and possibly even their name) and every time we do that it will hurt you and everyone else using these types.

I have an alternative suggestion that might hurt a lot less in the future: What do you think about wrapping the Return Type of createSlice?

That would even be composable with multiple ways of extending it:

import {createSlice, Slice} from '@reduxjs/toolkit';


export function enhanceSlice<S extends Slice<any,any>>(
  slice: S,
  advancedOptions: any = {},
) {
  const { test } = advancedOptions;
  
  return {
    ...slice,
    test,
  };
}


const x = enhanceSlice(createSlice({
  name: "test",
  initialState: 0,
  reducers: {}
}), {
  test: "asd"
})

@wuifdesign
Copy link
Author

it would be ok if the types change. i know if i implement another library like that an update can effect my implementation. it is ok, if the options change.

It would be better to have types comming from the libary as copying it to my code, as i copy it, i will not get a type error even if the options changed and the ones i currently provide didn't match the current ones. if i copy them i have to check them on each update if they match the current from the library.

@phryneas
Copy link
Member

Phew. I'm pretty torn here.

@markerikson, what are your thoughts on exporting a copy of those types as __INTERNAL_UNSTABLE_RestrictCaseReducerDefinitionsToMatchReducerAndPrepare etc.?

@markerikson
Copy link
Collaborator

I would really rather not export those.

Couldn't you do some kind of ReturnType<typeof createSlice> / Parameters<typeof createSlice> thing to extract the types as needed?

@phryneas
Copy link
Member

That won't work, as the parameters are dependent on Generics - ReturnType always returns a "flat" type with all "generic-ity" removed.

@phryneas
Copy link
Member

So, @wuifdesign, as you see, we both aren't feeling good on this one.
This is something we might reconsider after we reach a point where we can't think of any features to be added and thus assuming that those typings are stable, but I think we're not going to export those types for now.

You might - for now - actually be better of just copying those types into your code base, because the shape of those objects is not going to change anytime soon - just the types describing that shape. So if you copy the types out, you probably won't get a lot less conflicts than if you were to reference our types.

Although I'd again suggest to go for a "wrapping the result" style instead of a "wrapping the function" style, as I suggested in #276 (comment) - that won't require any knowledge of our internal types at all.

@markerikson
Copy link
Collaborator

Yeah. Given that this isn't something we plan to do at the moment, I'll close this.

@phryneas
Copy link
Member

phryneas commented Jan 2, 2020

Hey @wuifdesign - the type refactor is through and we now felt confident to export some of those types.

here's how to use them - I hope it helps you!

@wuifdesign
Copy link
Author

@phryneas thx, i already saw it, and i will try it after holidays

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