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.

Sign up for GitHub

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

create-subscription #12325

Changes from 1 commit
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
Prev Previous commit
Next Next commit
Fixed example imports in README
  • Loading branch information
@bvaughn
bvaughn authored Mar 8, 2018
commit a2f43a57714557b7c712cdcc3bbb4230025ae232
8 changes: 4 additions & 4 deletions packages/create-subscription/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ npm install create-subscription --save
To configure a subscription, you must specify three properties: `getValue`, `subscribe`, and `unsubscribe`.

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 createComponent from "create-subscription";
import { createSubscription } from "create-subscription";

const Subscription = createComponent({
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.

Expand Down Expand Up @@ -73,7 +73,7 @@ Below is an example showing how `create-subscription` can be used to subscribe t

```js
import React from "react";
import createComponent from "create-subscription";
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.
Expand Down Expand Up @@ -147,7 +147,7 @@ Below is an example showing how `create-subscription` can be used with native Pr

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

// Start with a simple component.
function LoadingComponent({ loadingStatus }) {
Expand Down Expand Up @@ -187,4 +187,4 @@ const PromiseSubscription = createComponent({
<PromiseSubscription source={loadingPromise}>
{loadingStatus => <LoadingComponent loadingStatus={loadingStatus} />}
</PromiseSubscription>
```
```