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

Minimal implementation of stateless components #4587

Merged
merged 2 commits into from Aug 11, 2015

Conversation

Projects
None yet
6 participants
@sophiebits
Copy link
Contributor

sophiebits commented Aug 8, 2015

Stateless pure-function components give us more opportunity to make performance optimizations. For now, we'll do a minimal implementation which has similar performance characteristics to other components in the interests of shipping 0.14 and allowing people to begin writing code using this pattern; in the future we can refactor to allocate less and avoid other unnecessary work.

This adapts @vslinko's tests from #3995.

Deduplicate logic in ReactElementValidator
Shouldn't be much change. Notably, this calls `.getName()` instead of trying to derive it manually, which is more consistent.
@sophiebits

This comment has been minimized.

Copy link
Contributor

sophiebits commented Aug 8, 2015

cc @sebmarkbage

@vslinko: Thanks for your work in #3995. We're planning to go with a minimal implementation for now but let's keep your other PR open to see if we want to pull it in for 0.15.

Minimal implementation of stateless components
Stateless pure-function components give us more opportunity to make performance optimizations. For now, we'll do a minimal implementation which has similar performance characteristics to other components in the interests of shipping 0.14 and allowing people to begin writing code using this pattern; in the future we can refactor to allocate less and avoid other unnecessary work.

@sophiebits sophiebits force-pushed the sophiebits:stateless-fn branch to 5a7bd96 Aug 8, 2015

@vslinko

This comment has been minimized.

Copy link

vslinko commented Aug 9, 2015

@spicyj ok, agreed

@sebmarkbage

This comment has been minimized.

Copy link
Member

sebmarkbage commented Aug 11, 2015

lgtm

sophiebits added a commit that referenced this pull request Aug 11, 2015

Merge pull request #4587 from spicyj/stateless-fn
Minimal implementation of stateless components

@sophiebits sophiebits merged commit d1a2193 into facebook:master Aug 11, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
inst = new Component(publicProps, publicContext, ReactUpdateQueue);
}

if (inst === null || inst === false || ReactElement.isValidElement(inst)) {

This comment has been minimized.

@sebmarkbage

sebmarkbage Aug 11, 2015

Member

This won't work because the construct trap will intercept any non-object and turn it into this. :(

@sebmarkbage sebmarkbage referenced this pull request Aug 11, 2015

Closed

Umbrella 0.14 #3220

20 of 25 tasks complete

@alexeyraspopov alexeyraspopov referenced this pull request Sep 8, 2015

Merged

0.14 RC blog post #4797

@epsitec

This comment has been minimized.

Copy link

epsitec commented Nov 14, 2015

@spicyj I'm not sure: are there plans for the stateless function component to be handled as being pure and thus avoiding repeated calls (to render the output) if the properties provided stay unchanged (with respect to a shallowCompare, probably)?

@jimfb

This comment has been minimized.

Copy link
Contributor

jimfb commented Nov 14, 2015

@epsitec You can read the relevant discussion is here: #3995 (comment)

@sebmarkbage

This comment has been minimized.

This is an unfortunate check. Been try to remove all prototype assumptions so that we can switch to simple functions style. I don't have any better suggest atm. :/

This comment has been minimized.

Copy link
Contributor

sophiebits replied Mar 28, 2016

What do you mean? This is all internal anyway.

This comment has been minimized.

Copy link
Member

sebmarkbage replied Mar 28, 2016

Yea, but IMO we should stop using classes for internal instances and switch to pattern matching only where needed. That pattern matching would like include some nominal type information that could power this check.

This comment has been minimized.

Copy link
Contributor

sophiebits replied Mar 28, 2016

Sure. We can change to a flag on the internal instance, at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment