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

#58 Added wrappedInstance to the meta object #139

Merged
merged 2 commits into from
Jul 28, 2016

Conversation

eyalw
Copy link
Contributor

@eyalw eyalw commented Jul 23, 2016

Added wrappedInstance to the meta object for side effects during then() and andThen().

the wrappedInstance is called just "component" in the context of then() because I thought it would be a simpler name yet clear enough.

@eyalw
Copy link
Contributor Author

eyalw commented Jul 23, 2016

fixes #58

@@ -279,6 +279,7 @@ function connect(mapPropsToRequestsToProps, defaults, options) {
} else {
const request = mapping.buildRequest(mapping)
meta.request = request
meta.component = this.refs.wrappedInstance
Copy link
Contributor

Choose a reason for hiding this comment

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

this should probably be done right after line 267 (or something like const meta = Object.assign(meta, {component: this.refs.wrappedInstance}) so that identity requests and regular fetches (i.e. the branches of the if/else) are treated the same in this regard.

@ryanbrainard
Copy link
Contributor

Thanks for opening this. Would you also be able to add a test (there's probably an existing one that can be easily modified), add it to the API doc, and perhaps add an example?

@@ -22,6 +22,7 @@ Instead, it *returns* a new, connected component class, for you to use.
- `force` *(Boolean)*: Forces the data to be always fetched when new props are received. Takes precedence over `comparison`.
- `comparison` *(Any)*: Custom value for comparing this request and the previous request when the props change. If the `comparison` values are *not* strictly equal, the data will be fetched again. In general, it is preferred to rely on the default that compares material changes to the request (i.e. URL, headers, body, etc); however, this is helpful in cases where the request should or should not be fetched again based on some other value. If `force` is true, `comparison` is not considered.
- `then(value, meta): request` *(Function)*: returns a request to fetch after fulfillment of this request and replaces this request. Takes the `value` and `meta` of this request as arguments. Return `undefined` in `then` to do side-effects after a successful request, leaving the request as is.
- `meta` is an object with keys `{ request, response, component, ...<your meta>}` where `component` is equal to the wrappedInstance. You can use it to create side effects on promise fulfillment. e.g. `then(value, meta) { meta.component.onDataLoaded(value); }`
Copy link
Contributor

Choose a reason for hiding this comment

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

only if you passed withRef: true to options, otherwise component will be undefined

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point. I'll add this note after merging.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point. I'll add this note after merging.

@ryanbrainard
Copy link
Contributor

Thanks @eyalw !

@ryanbrainard ryanbrainard merged commit 70ab13c into heroku:master Jul 28, 2016
@ryanbrainard
Copy link
Contributor

I was just doing a quick test of this in a real app and got:

Invariant Violation: addComponentAsRefTo(...): Only a ReactOwner can have refs. You might be adding a ref to a component that was not created inside a component's `render` method, or you have multiple copies of React loaded

I haven't taken the time to dig in, but just wanted to check with @eyalw if you had this happen when using connect.options({ withRef: true })

@eyalw
Copy link
Contributor Author

eyalw commented Jul 28, 2016

I didn't, an odd message. Do you have a short code that duplicated the
warning?
On Thu, 28 Jul 2016 at 5:31 AM Ryan Brainard notifications@github.com
wrote:

I was just doing a quick test of this in a real app and got:

Invariant Violation: addComponentAsRefTo(...): Only a ReactOwner can have refs. You might be adding a ref to a component that was not created inside a component's render method, or you have multiple copies of React loaded

I haven't taken the time to dig in, but just wanted to check with @eyalw
https://github.com/eyalw if you had this happen when using connect.options({
withRef: true })


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#139 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAlJnV-V6zyPNultk4v0Ki9rI8FU9o2gks5qaBSHgaJpZM4JTXPY
.

@eyalw
Copy link
Contributor Author

eyalw commented Sep 2, 2016

@ryanbrainard are you releasing a new beta with this code soon?

@ryanbrainard
Copy link
Contributor

@eyalw Oh, yes, sorry, I can cut a release

@ryanbrainard
Copy link
Contributor

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

3 participants