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

Safe plugin #36

Closed
wants to merge 2 commits into from
Closed

Safe plugin #36

wants to merge 2 commits into from

Conversation

bcherny
Copy link
Owner

@bcherny bcherny commented May 27, 2018

This PR introduces the SafePlugin type and createSafeStore utility. They opt the user into a more restricted subset of Undux that is more aggressive about preventing cycles in Effects. The way it does this is by statically restricting calls to store.set to React components only (this means you can't call store.set from Effects).

This makes Undux less powerful by imposing a constraint on data flow similar to that imposed by Redux and Flux. It's analogous to a Redux/Flux constraint that prevents calling dispatcher.dispatch outside of a React component, or from within a dispatch call. This is not a safety mode that's useful for most users; cycles in Rx streams are rare, and may be better prevented by other means. This PR is a first pass to sketch out what the experience might feel like.

Future directions to explore:

  • Be more permissive: only ban calling .store.set from Effects, and allow everywhere else
  • Be more permissive: use a linter to prevent cycles in Effects, but allow store.set calls in general
  • Be more permissive: perform dynamic analysis to warn about cycles in Effects, but allow store.set calls in general perform dynamic analysis to warn about cycles in Effects #37
  • Also enforce this constraint with a runtime check

Usage

-const {createStore} = require('undux');
+const {createSafeStore} = require('undux');

-import type {Plugin} from 'undux';
+import type {SafePlugin} from 'undux';

@hurrymaplelad
Copy link

I'm curious how you'd wire up a request without set in effects. Normally I'd do something like:

  store.on('params').subscribe((p) => {
    store.set('response')(null);
    fetch(makeUrl(p)).then((response) => {
      store.set('response')(response);
    });
  });

@bcherny
Copy link
Owner Author

bcherny commented May 31, 2018

@hurrymaplelad With this proposed architecture, a lot of things that normally live in effects would live in action creators (aka. utility functions) instead. This makes Undux look a lot like Flux, but typesafe. I'm not convinced this is a good direction to go in; effects give order and meaning to action creators via RxJS's stream algebra, making it possible to sequence, compose, statically analyze, and otherwise combine actions. If we move actions back to action creators, we get rid of this set of features.

So your code might become something like:

Effects.js

// Empty

Actions.js

function requestData(store: Store) {
  return () =>
    fetch(makeUrl(store.get('params')))
      .then(store.set('response'))
}

Component.react.js

<button onClick={requestData(this.props.store)}>Send request</button>

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

Successfully merging this pull request may close these issues.

None yet

2 participants