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

On effects, this.reducer() is not correctly infered #870

Closed
semoal opened this issue Feb 25, 2021 · 14 comments
Closed

On effects, this.reducer() is not correctly infered #870

semoal opened this issue Feb 25, 2021 · 14 comments
Assignees
Milestone

Comments

@semoal
Copy link
Member

semoal commented Feb 25, 2021

Describe the bug

With Rematch you can use this inside effects to dispatch data to reducers.

Basically this:

export const players = createModel<RootModel>()({
	state: {
		players: [],
	},
	reducers: {
		SET_PLAYERS: (state, players) => {
			return {
				...state,
				players,
			}
		},
	},
	effects: () => ({
    async getPlayers() {
      const response = await fetch(
        'https://www.balldontlie.io/api/v1/players'
      )
      const { data } = await response.json()
      this.SET_PLAYERS(data)
    }
  })
})

is equivalent to this:

export const players = createModel<RootModel>()({
	state: {
		players: [],
	},
	reducers: {
		SET_PLAYERS: (state, players) => {
			return {
				...state,
				players,
			}
		},
	},
	effects: (dispatch) => ({
    async getPlayers() {
      const response = await fetch(
        'https://www.balldontlie.io/api/v1/players'
      )
      const { data } = await response.json()
      dispatch.players.SET_PLAYERS(data)
    }
  })
})

To Reproduce

When using Typescript, this.SET_PLAYERS() won't get autocompleted because this isn't overriden by Rematch.

Current work around

      (this as typeof players).SET_PLAYERS(data)

Expected behavior

Should autocomplete reducers and effects of the current Model.

Additional context

  • Version: 2.0.1
@maciekgrzybek
Copy link

I'm happy to take a look at this :)

@maciekgrzybek
Copy link

It's really tough to figure it out really... Do you have any ideas what might be causing that?

@semoal
Copy link
Member Author

semoal commented Apr 20, 2021

Actually this type:

export type ModelEffect<
	TModels extends Models<TModels> = Record<string, any>
> = (
	payload: Action['payload'],
	rootState: RematchRootState<TModels>,
	meta: Action['meta']
) => any

Is the current type for an effect, you will see that if you add the this property with anything else, you get autocomplete of the rootstate:

export type ModelEffect<
	TModels extends Models<TModels> = Record<string, any>
> = (
    this: RematchRootState<TModels>
	payload: Action['payload'],
	rootState: RematchRootState<TModels>,
	meta: Action['meta']
) => any

We must add something like: ExtractReducers&Effects<TModel> where we can pass a Model and extract his effects and reducers.

It is not an easy fix, @tianzhich it's an expert with types and made something impossible to practically any library out there, is that being practically 100% compatible with the current Redux and React Redux typings.

@maciekgrzybek
Copy link

Awesome, thanks for explanation, I'll give it another go :)

@maciekgrzybek
Copy link

I have another question :) I'm not sure if I'm using the correct local examples to actually confirm my changes in the core? is it just the examples folder? Is it linked automatically to the local rematch? Unfortunately CONTRIBUTING.md file is not really specific about it :(

@semoal
Copy link
Member Author

semoal commented Apr 22, 2021

Yes the examples folder is linked via the monorepo.
You only need to re-run the build on the package you change.

yarn build --scope @rematch/core

In the root of the project and will rebuild in less than 5 seconds the package =)

@semoal
Copy link
Member Author

semoal commented Apr 22, 2021

We're using tsdx internally for the packages, so probably you could use also tsdx watch inside the core package (not sure about this)
Tomorrow I'll improve the CONTRIBUTION.md =)

@maciekgrzybek
Copy link

Wow, that was a quick response :D Thanks a lot :)

@semoal semoal added this to the 2.1.0 milestone Apr 27, 2021
@semoal semoal mentioned this issue May 16, 2021
15 tasks
@tianzhich
Copy link
Collaborator

Hi mate, how about the progress of this? @maciekgrzybek I can continue from your work if you did some modifications.

@maciekgrzybek
Copy link

Hey @tianzhich I totally dropped the ball on this one, I had few approaches but couldn't figure it out at all :(

@tianzhich
Copy link
Collaborator

I created a minimal rematch typing system and tried to fix this but failed. Maybe someone can pick this one and continue from here.

@semoal
Copy link
Member Author

semoal commented Jun 15, 2021

Close this, since will be fixed with 2.1.0 release

@semoal semoal closed this as completed Jun 15, 2021
@ardyfeb
Copy link

ardyfeb commented Oct 18, 2021

Did this issue fixed ? i'm still not getting correct typing on this inside effects function

@semoal
Copy link
Member Author

semoal commented Oct 18, 2021

It is partially, we couldn’t fix a 100% type checking it shouldn’t return error at least. Contributions are appreciated ;)

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

No branches or pull requests

4 participants