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

create-subscription #12325

Merged
merged 54 commits into from
Mar 13, 2018
Merged
Show file tree
Hide file tree
Changes from 47 commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
d5d8bf6
POC for create-component-with-subscriptions
bvaughn Mar 5, 2018
7b1e8c2
Updated README
bvaughn Mar 5, 2018
f8743b3
Added Rollup bundle
bvaughn Mar 5, 2018
4304b55
Expanded tests
bvaughn Mar 5, 2018
30eb16a
Updated bundle comment
bvaughn Mar 5, 2018
eb1372c
Added a test for "cold" observable
bvaughn Mar 5, 2018
88e7e22
Updated inline comments
bvaughn Mar 5, 2018
c395051
Updated README examples
bvaughn Mar 5, 2018
78c9a4c
Added a test (and README docs) for Promises
bvaughn Mar 5, 2018
d1fc6e8
Added a caveat about Promises to README
bvaughn Mar 5, 2018
2e98ca9
Use a HOC for functional components and a mixin for ES6 components
bvaughn Mar 6, 2018
5093ab4
Added tests for create-react-class
bvaughn Mar 6, 2018
54468e8
Flow fix
bvaughn Mar 6, 2018
6b16b7c
Added a ref test
bvaughn Mar 6, 2018
d05ffa3
Added a test for react-lifecycles-compat
bvaughn Mar 6, 2018
bd36fb4
Updated README to show class component
bvaughn Mar 6, 2018
a11164e
Added docs for default values
bvaughn Mar 6, 2018
31edf59
Improved README examples
bvaughn Mar 6, 2018
256c5e5
Simplified Promise docs and added additional test
bvaughn Mar 6, 2018
6dcac15
Swapped functional/class component usage in examples
bvaughn Mar 6, 2018
39d7ba8
Split internal and public API tests
bvaughn Mar 6, 2018
b5571c1
Tweaks
bvaughn Mar 6, 2018
2192fd5
Changed impl to only support one subscription per component
bvaughn Mar 6, 2018
fdfa22b
Docs tweak
bvaughn Mar 6, 2018
afeb6cd
Docs tweaks
bvaughn Mar 6, 2018
7532184
Refactored create-subscription to more closely mimic context API
bvaughn Mar 7, 2018
0f936ba
Renamed create-component-with-subscriptions => create-subscription
bvaughn Mar 7, 2018
2d824c2
Renamed references to create-subscription
bvaughn Mar 7, 2018
3edff49
Replaced .toThrow with .toWarnDev
bvaughn Mar 7, 2018
e056172
Disable render-phase side effects
bvaughn Mar 7, 2018
9bdc6d6
Updated docs
bvaughn Mar 7, 2018
9ffe079
README and naming tweaks
bvaughn Mar 7, 2018
629f145
README tweaks
bvaughn Mar 7, 2018
48b4a1b
Wording tweak
bvaughn Mar 7, 2018
ee2ae93
Inline comments tweak
bvaughn Mar 7, 2018
64d80b8
Minor test tidying up
bvaughn Mar 7, 2018
ad190fb
Added more context to README intro
bvaughn Mar 7, 2018
3288726
Wordsmith nit picking
bvaughn Mar 7, 2018
81f2695
Wordsmith nit picking
bvaughn Mar 7, 2018
267a76b
Replaced Value with Value | void type
bvaughn Mar 7, 2018
db7b84f
Tweaks in response to Flarnie's feedback
bvaughn Mar 7, 2018
32d6d40
Added RxJS for tests instead of fake impls
bvaughn Mar 7, 2018
5557120
Improved children Flow type slightly
bvaughn Mar 7, 2018
ee3dfcc
Added Flow <> around config
bvaughn Mar 7, 2018
a2f43a5
Fixed example imports in README
bvaughn Mar 8, 2018
4e57ed7
Replaced createComponent() references with createSubscription() in RE…
bvaughn Mar 8, 2018
f0c68b8
Changed subscribe() to return an unsubscribe method (or false)
bvaughn Mar 8, 2018
63a65e6
Flow type tweak
bvaughn Mar 12, 2018
e6740aa
Merge branch 'master' into create-component-with-subscriptions
bvaughn Mar 13, 2018
c116528
Responded to Andrew's PR feedback
bvaughn Mar 13, 2018
c1dd9a7
Docs updatE
bvaughn Mar 13, 2018
e10e2fc
Flow tweak
bvaughn Mar 13, 2018
f03dfa9
Addressed PR feedback from Flarnie
bvaughn Mar 13, 2018
6f740d9
Removed contradictory references to Flux stores in README
bvaughn Mar 13, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
186 changes: 186 additions & 0 deletions packages/create-subscription/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
# create-subscription

[Async-safe subscriptions are hard to get right.](https://gist.github.com/bvaughn/d569177d70b50b58bff69c3c4a5353f3)

`create-subscription` provides an simple, async-safe interface to manage a subscription.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one line summary is too appealing. It doesn't suggest any reasons for why I shouldn't use subscriptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha. I tried to clarify who shouldn't use it below (in the "Who should use this?" section). Any specific wording suggestions?


## Who should use this?

This utility is should be used for subscriptions to a single value that are typically only read in one place and may update frequently (e.g. a component that subscribes to a geolocation API to show a dot on a map).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit - 'This utility is should' -> 'This utility should'


Other cases have better long-term solutions:
* Redux/Flux stores should use the [context API](https://reactjs.org/docs/context.html) instead.
* I/O subscriptions (e.g. notifications) that update infrequently should use [`simple-cache-provider`](https://github.com/facebook/react/blob/master/packages/simple-cache-provider/README.md) instead.
* Complex libraries like Relay/Apollo should use this same technique (as referenced [here](https://gist.github.com/bvaughn/d569177d70b50b58bff69c3c4a5353f3)) in a way that is most optimized for their library usage.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit - this bullet could be reworded for clarity, something like this:

  • Complex libraries like Relay/Apollo should manage subscriptions manually with the same techniques which this library uses under the hood (as referenced ..)) in a way that is most optimized for their library usage.


## What types of subscriptions can this support?

This abstraction can handle a variety of subscription types, including:
* Event dispatchers like `HTMLInputElement`.
* Custom pub/sub components like Relay's `FragmentSpecResolver`.
* Observable types like RxJS `BehaviorSubject` and `ReplaySubject`. (Types like RxJS `Subject` or `Observable` are not supported, because they provide no way to read the "current" value after it has been emitted.)
* Native Promises.

# Installation

```sh
# Yarn
yarn add create-subscription

# NPM
npm install create-subscription --save
```

# Usage

To configure a subscription, you must provide two methods: `getValue` and `subscribe`.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YES! I was going to suggest starting with a concrete example, glad you added this here.

```js
import { createSubscription } from "create-subscription";

const Subscription = createSubscription({
getValue(source) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a while, I thought that this function was supposed to be getInitialValue until I read the implementation. Now I understand that you're handling this case.

  1. Get the initial value for state
  2. Wait to mount
  3. Value changed while you were waiting to mount
  4. Subscribe
  5. The subscription isn't going to vend you the changed value, so you getValue while subscribing.

I'm worried people might do this, though I don't know what to do about it.

const Subscription = createSubscription({
  getValue(source) {
    return SomeConfig.initialValue;
  },
  subscribe(source, callback) {
    const {dispose} = source.listen(value => callback(value));
    return dispose;
  },
});

<Subscription source={someWebsocketOrSomething}>
  ...
</Subscription>

…where the source isn't something that offers a way to read the value synchronously.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any actionable suggestion to avoid this? Docs? Method names?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we call it 'getCurrentValue'?

We could also add a note in the comments saying that this value will be read repeatedly as the component re-renders.

// Return the current value of the subscription (source),
// or `undefined` if the value can't be read synchronously (e.g. native Promises).
},
subscribe(source, callback) {
// Subscribe (e.g. add an event listener) to the subscription (source).
// Call callback(newValue) whenever a subscription changes.
// Return an unsubscribe method,
// Or false if unsubscribe is not supported (e.g. native Promises).
}
});
```

To use the `Subscription` component, pass the subscribable property (e.g. an event dispatcher, Flux store, observable) as the `source` property and use a [render prop](https://reactjs.org/docs/render-props.html), `children`, to handle the subscribed value when it changes:

```js
<Subscription source={eventDispatcher}>
{value => <AnotherComponent value={value} />}
</Subscription>
```

# Examples

This API can be used to subscribe to a variety of "subscribable" sources, from Flux stores to RxJS observables. Below are a few examples of how to subscribe to common types.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"from Flux stores" — this clashes with what you say earlier (this shouldn't be used for Flux stores)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good call.


## Subscribing to event dispatchers

Below is an example showing how `create-subscription` can be used to subscribe to event dispatchers such as DOM elements or Flux stores.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same


```js
import React from "react";
import { createSubscription } from "create-subscription";

// Start with a simple component.
// In this case, it's a functional component, but it could have been a class.
function FollowerComponent({ followersCount }) {
return <div>You have {followersCount} followers!</div>;
}

// Create a wrapper component to manage the subscription.
const EventHandlerSubscription = createSubscription({
getValue: eventDispatcher => eventDispatcher.value,
subscribe: (eventDispatcher, callback) => {
const onChange = event => callback(eventDispatcher.value);
eventDispatcher.addEventListener("change", onChange);
return () => eventDispatcher.removeEventListener("change", onChange);
}
});

// Your component can now be used as shown below.
// In this example, 'eventDispatcher' represents a generic event dispatcher.
<EventHandlerSubscription source={eventDispatcher}>
{value => <FollowerComponent followersCount={value} />}
</EventHandlerSubscription>;
```

## Subscribing to observables

Below are examples showing how `create-subscription` can be used to subscribe to certain types of observables (e.g. RxJS `BehaviorSubject` and `ReplaySubject`).

**Note** that it is not possible to support all observable types (e.g. RxJS `Subject` or `Observable`) because some provide no way to read the "current" value after it has been emitted.

### `BehaviorSubject`
```js
const BehaviorSubscription = createSubscription({
getValue: behaviorSubject => behaviorSubject.getValue(),
subscribe: (behaviorSubject, callback) => {
const subscription = behaviorSubject.subscribe(callback);
return () => subscription.unsubscribe();
}
});
```

### `ReplaySubject`
```js
const ReplaySubscription = createSubscription({
getValue: replaySubject => {
let currentValue;
// ReplaySubject does not have a sync data getter,
// So we need to temporarily subscribe to retrieve the most recent value.
replaySubject
.subscribe(value => {
currentValue = value;
})
.unsubscribe();
return currentValue;
},
subscribe: (replaySubject, callback) => {
const subscription = replaySubject.subscribe(callback);
return () => subscription.unsubscribe();
}
});
```

## Subscribing to a Promise

Below is an example showing how `create-subscription` can be used with native Promises.

**Note** that it an initial render value of `undefined` is unavoidable due to the fact that Promises provide no way to synchronously read their current value.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firstly - What do you think of showing an example that passes through the value returned by the promise? Like so:

const PromiseSubscription = createComponent({
  getValue: //...
  subscribe: (promise, callback) => {
    promise.then(
      // Success
      (v) => callback(v),
      // Failure
      (v) => callback(v)
    ).;
  },
// ...
});

Secondly:

Note that it an initial render value of undefined is unavoidable due to the fact that Promises provide no way to synchronously read their current value.

True; unless you're overly clever! :D

let savedPromiseValue;
const PromiseSubscription = createComponent({
  getValue: promise => {
    // we have saved the promise value from when it was last updated
    return savedPromiseValue;
  },
  subscribe: (promise, callback) => {
    promise.then(
      // Success
      (v) => {
        savedPromiseValue = v;
        callback(v);
      },
      // Failure
      (v) => callback(v)
    );
  },
// ...
});

Copy link
Contributor Author

@bvaughn bvaughn Mar 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think of showing an example that passes through the value returned by the promise?

I show true/false because the example shows a "loading" promise.

Maybe I can compromise? Check out the commit I'll push shortly.

True; unless you're overly clever! :D

Unfortunately, that wouldn't prevent the initial render with undefined because we don't "subscribe" until the commit phase. (Unless I'm misunderstanding something.)


**Note** the lack of a way to "unsubscribe" from a Promise can result in memory leaks as long as something has a reference to the Promise. This should be taken into considerationg when determining whether Promises are appropriate to use in this way within your application.

```js
import React from "react";
import { createSubscription } from "create-subscription";

// Start with a simple component.
function LoadingComponent({ loadingStatus }) {
if (loadingStatus === undefined) {
// Loading
} else if (loadingStatus === null) {
// Error
} else {
// Success
}
}

// Wrap the functional component with a subscriber HOC.
// This HOC will manage subscriptions and pass values to the decorated component.
// It will add and remove subscriptions in an async-safe way when props change.
const PromiseSubscription = createSubscription({
getValue: promise => {
// There is no way to synchronously read a Promise's value,
// So this method should return undefined.
return undefined;
},
subscribe: (promise, callback) => {
promise.then(
// Success
value => callback(value),
// Failure
() => callback(null)
);

// There is no way to "unsubscribe" from a Promise.
// create-subscription will still prevent stale values from rendering.
return false;
}
});

// Your component can now be used as shown below.
<PromiseSubscription source={loadingPromise}>
{loadingStatus => <LoadingComponent loadingStatus={loadingStatus} />}
</PromiseSubscription>
```
12 changes: 12 additions & 0 deletions packages/create-subscription/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
* Copyright (c) 2013-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

'use strict';

export * from './src/createSubscription';
7 changes: 7 additions & 0 deletions packages/create-subscription/npm/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
'use strict';

if (process.env.NODE_ENV === 'production') {
module.exports = require('./cjs/create-subscription.production.min.js');
} else {
module.exports = require('./cjs/create-subscription.development.js');
}
21 changes: 21 additions & 0 deletions packages/create-subscription/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
"name": "create-subscription",
"description": "HOC for creating async-safe React components with subscriptions",
"version": "0.0.1",
"repository": "facebook/react",
"files": [
"LICENSE",
"README.md",
"index.js",
"cjs/"
],
"dependencies": {
"fbjs": "^0.8.16"
},
"peerDependencies": {
"react": "16.3.0-alpha.1"
},
"devDependencies": {
"rxjs": "^5.5.6"
}
}
Loading