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

Enables getting the current state #60

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

kabriel
Copy link

@kabriel kabriel commented Oct 12, 2018

I need access to the current state outside of the render tree so I added a bit of code to store and retrieve it. Maybe others are running into the same requirement.

@j-f1
Copy link

j-f1 commented Oct 13, 2018

Perhaps getState instead of getCurrent?

@kabriel
Copy link
Author

kabriel commented Oct 13, 2018

Ya, that is reasonable. I wanted it to be clear that this was a single snapshot/immutable point-in-time of the state, and not a reference to it that would update, but I don’t feel that strongly.

@j-f1
Copy link

j-f1 commented Oct 13, 2018

That’s the kind of thing that would be best described by documentation IMO.

@aweary
Copy link
Owner

aweary commented Oct 15, 2018

This is actually how state used to managed, but I changed it in #43. Tracking state outside of React can make things complicated when it comes to async/concurrent rendering. With this implementation we could run the risk of currentState being out of sync with the actual state React is rendering with.

Can you describe your use case in a little more detail? I usually recommend reading state via React, and then syncing with external services in the commit phase (componentDidMount, componentDidUpdate). It's a bit convoluted because of how you can access Context, but it would look something like:

class Subscription extends React.Component {
  componentDidMount() {
    this.updateExternalState();
  }

  componentDidUpdate() {
    this.updateExternalState();
  }

  // Updates some external (non-React) code with the
  // new value(s) read from state in Connected.
  syncWithExternalState = () => {
    const { value } = this.props;
    updateExternalState(value);
  };

  render() {
    return null;
  }
}

const Connected = () => (
  <Consumer select={[selectName]}>
    {name => <Subscription value={name} />}
  </Consumer>
);

It's verbose, but I suspect this will get easier in the future as the React team works on this class of problems.

@kabriel
Copy link
Author

kabriel commented Oct 15, 2018

The key distinction here i believe is in the use of the word tracking. My needs (and this change) are to get a point-in-time snapshot of the current state, and specifically not to track the state's change. I am using this for code that is well outside the React rendering process, such as with callbacks that are part of external side-effects: "i just got an event from an external system and i need to take action in-part based on the current-state". This is not about knowing when the state changes, it is just about getting the current value and making a point-in-time decision.

I understand the concern with exposing this value externally, which is why i thought calling it getCurrent could help with the distinction. Maybe it could even be called getStateSnapshot or something more obvious. If feels like, in any case, knowing the current state easily without having to be in a callback from React is important for some reasonable use-cases.

@brianc
Copy link

brianc commented Oct 25, 2018

Love this library. It works amazingly well with typescript & makes doing simple fluxy apps so quick and straight-forward.

fwiw I ran into this very recently. When the app starts up I run a fetch request to get the user info & and put it into state:

example:

// actions.js
async function loadUser() {
  const res = await fetch('/current-user')
  const user = await res.json()
  mutate(state => (state.user = user))
}

Later I want to do another fetch somewhere else using the stored user information. I would have to pass the user into the action, but what I want is another action like this:

// actions.js
...
async function doSomethingCoolWithCurrentUser() {
  const { user } = getState()
  await fetch(`/users/${user.id}/something-cool')
}

but you can't do that...

you can do this:

async function doSomethingCoolWithCurrentUser() {
  let userId;
  mutate(state => (userId = state.user.id))
  await fetch(`/users/${userId}/something-cool')
}

but that is kinda gross. Also IIRC if you were to do something like:

async function doSomethingCoolWithCurrentUser() {
  let user;
  mutate(state => (user = state.user))
  await fetch(`/users/${user.id}/something-cool')
}

it gets weird because user is actually a reference to the proxy and not the original object.

It's vaguely similar to redux-thunk's getState() thing it injects into your action creators.

I was thinking of forking & adding it to try it out but then see this PR is already open so I just wanted to throw on a little cheer for it. 👏

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.

4 participants