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

Consider adding `safe` effect wrapper #1250

Open
Andarist opened this Issue Nov 5, 2017 · 7 comments

Comments

Projects
None yet
5 participants
@Andarist
Copy link
Member

Andarist commented Nov 5, 2017

If somebody wants to work in it - let me know. We can then establish how this would work

@Andarist Andarist added the enhancement label Nov 5, 2017

@Andarist Andarist added this to the v1 milestone Nov 5, 2017

@Andarist Andarist modified the milestones: v1, post-v1 Nov 8, 2017

@madhums

This comment has been minimized.

Copy link

madhums commented Nov 12, 2017

@Andarist could you describe what is the idea behind "safe" effect?

@Andarist

This comment has been minimized.

Copy link
Member Author

Andarist commented Nov 12, 2017

Im not yet exactly sure about the API, but one idea is to:

const safe = effect => (effect[SAFE] = true)

function* someSaga() {
  // where result is of type Error or Result
  const result = yield safe(call(someAPI, arg1))
  // in case of an error would die gracefully instead of failing the parent 
  yield safe(fork(otherSaga, arg2))
}

In this variant SAFE would have to be interpreted in the redux-saga's core.

Other way of implementing it would be require implementing equivalent of:

function* safe(effect) {
	try {
		return { result: yield effect }
	} catch (err) {
		return { err }
	}
}

In this variant fork would have to be treated differently, so the task descriptor could get returned to the caller, instead of blocking.

@SeregaSE

This comment has been minimized.

Copy link
Contributor

SeregaSE commented Jul 11, 2018

I can try do this

@Andarist

This comment has been minimized.

Copy link
Member Author

Andarist commented Jul 24, 2018

Cool!

Seems to be that second variant of what I have proposed (no SAFE symbol, but rather just a simple wrapper around yield effect) is what we should implement.

@restrry

This comment has been minimized.

Copy link
Collaborator

restrry commented Jul 26, 2018

I'm using next pattern in my projects, that's kinda 2nd version

function* safe(effect) {
  try {
    return { result: yield effect, error: null }
  } catch (error) {
    return { result: null, error }
  }
}
@younesmln

This comment has been minimized.

Copy link

younesmln commented Sep 4, 2018

@Andarist interested in working on this enhancement

@Andarist

This comment has been minimized.

Copy link
Member Author

Andarist commented Sep 4, 2018

@younesmln go ahead, if you need any help - please just ask

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