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

Added an optional instance id property #1105

Merged
merged 4 commits into from
Aug 30, 2016
Merged

Added an optional instance id property #1105

merged 4 commits into from
Aug 30, 2016

Conversation

JevinAnderson
Copy link

This will allow synchronization of component id’s on both the server
and the client.

This will allow synchronization of component id’s on both the server
and the client.
@JevinAnderson
Copy link
Author

JevinAnderson commented Jul 19, 2016

[edit] Should close #1104

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 91.714% when pulling 8bbd0b7 on JevinAnderson:enhancement/Server-Side-rendering into 8c8f560 on JedWatson:master.

@JedWatson
Copy link
Owner

Good solution!

I don't think you need to add the instanceId to the state though. Defaulting to this in the componentWillMount method should be sufficient?

this._instancePrefix = 'react-select-' + (this.props.instanceId || ++instanceId) + '-';

Did you link to the wrong "should close" issue number btw?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.714% when pulling 9874b89 on JevinAnderson:enhancement/Server-Side-rendering into e19bce3 on JedWatson:master.

@JevinAnderson
Copy link
Author

I've updated my solution @JedWatson. Thanks for the suggestion.

@etburke
Copy link
Contributor

etburke commented Aug 12, 2016

We're also running into this. Thanks for working out a solution!

@JevinAnderson
Copy link
Author

@bruderstein @JedWatson

Any word on this?

@JedWatson
Copy link
Owner

Thanks for that @JevinAnderson, looks good now! Sorry for the delayed merge.

@JedWatson JedWatson merged commit 6e67933 into JedWatson:master Aug 30, 2016
@mverderese
Copy link

mverderese commented Aug 31, 2016

@JedWatson What is your workflow for rebuilding the dist files? I noticed that master is updated with this change, but if I do an npm install from master, the dist files are still outdated. I'd like to avoid forking if possible.

Thanks!

@JedWatson
Copy link
Owner

@mverderese I'm about to publish an rc with this fix in it, it should be fine if you update the dependency in your project to that.

@JevinAnderson JevinAnderson deleted the enhancement/Server-Side-rendering branch September 9, 2016 20:32
@gabrielflorit
Copy link

gabrielflorit commented Oct 24, 2016

I'm still getting Warning: React attempted to reuse markup in a container.... I'm on rc2 and I'm passing an instanceId to my component (like I believe @JevinAnderson suggested), e.g.

<Select instanceId='gabrielflorit' ... />

Am I doing something wrong?

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.

6 participants