Skip to content

Remove dependency on React internals#151

Open
tschaub wants to merge 1 commit into
onefinestay:masterfrom
tschaub:pure-render-mixin
Open

Remove dependency on React internals#151
tschaub wants to merge 1 commit into
onefinestay:masterfrom
tschaub:pure-render-mixin

Conversation

@tschaub

@tschaub tschaub commented Aug 5, 2016

Copy link
Copy Markdown

I'm hoping you'd be willing to move away from the dependency on react-addons-pure-render-mixin. Since this module requires functionality from React internals (react/lib/ReactComponentWithPureRenderMixin), it makes it so an application cannot use react-daterange-picker and load React from a CDN.

Since there is already a PureRenderMixin in this package, one alternative would be to use that. Or, since this does additional checks for moment, things could be split into one mixin for shallow comparisons with moment and one mixin for shallow comparisons without moment.

I'll be happy to make changes to this if you're interested in moving away from react-addons-pure-render-mixin.

@tschaub tschaub force-pushed the pure-render-mixin branch from eb08956 to 1824dba Compare August 5, 2016 02:38
@AlanFoster

Copy link
Copy Markdown
Member

I like the suggestion @tschaub 👍

Would you mind rebasing this and I can take a closer look at this too.

@tschaub tschaub force-pushed the pure-render-mixin branch from 1824dba to 0a60670 Compare August 11, 2016 03:56
@tschaub

tschaub commented Aug 11, 2016

Copy link
Copy Markdown
Author

@AlanFoster - rebased. Thanks for checking it out.

@tschaub

tschaub commented Aug 15, 2016

Copy link
Copy Markdown
Author

@AlanFoster - let me know if you think this should be handled another way.

@tschaub

tschaub commented Sep 23, 2016

Copy link
Copy Markdown
Author

Is there a way I can help get this in? Curious if anybody else is also interested in using React from a CDN (or separate bundle) and this date picker together.

@tschaub

tschaub commented Oct 18, 2016

Copy link
Copy Markdown
Author

It looks like this will even be more important for people upgrading React (when internal lib modules are moved or changed). See react/react#7770 (comment).

@gaearon

gaearon commented Oct 19, 2016

Copy link
Copy Markdown

To be fair, addons imported via packages will keep on working. If you don't use /lib/ explicitly in your code it's safe.

Still, I would suggest using React.PureComponent base class (new feature in React 15.3.0 and higher).

@AlanFoster

Copy link
Copy Markdown
Member

@tschaub I've been merging a lot of PRs for a 2.x release; Mind re-basing this, and we can get this shipped? 👍

@tschaub

tschaub commented Nov 17, 2016

Copy link
Copy Markdown
Author

@AlanFoster rebased (after a long delay).

This uses the existing util/PureRenderMixin in this repo. I didn't take time to make things work with React.PureComponent as suggested above, since I'm not sure if you want to depend on 15.3 (though that sounds like a good way to go).

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.

3 participants