Commit Graph

103 Commits

Author SHA1 Message Date
Andres Suarez 4c2f124aa8 Comment typo 2014-09-04 12:00:43 -04:00
Miorel Palii 335e91df71 Fix console warning in LegacyImmutableObject
It does check `hasOwnProperty`, but *after* accessing the field and therefore triggering enumerable getters in modified prototypes.
2014-08-29 14:32:50 -07:00
Cheng Lou b8ab95aaa7 Merge pull request #2054 from chenglou/accum
[RFC] Use `accumulateInto` to save even more allocation
2014-08-20 13:36:45 -07:00
Cheng Lou 48e901f8ae [RFC] Use accumulateInto to save even more allocation
Trying to make the event a bit more performant for events.

Feel free to reject this because the API inevitably isn't great. It's good for perf though, and since we're only using `accumulate` in very restrained places, I think we're fine.

`accumulateInto` is `accumulate` that mutates more and allocates less. I kept `accumulate` in case we want that in the future.
2014-08-20 13:10:50 -07:00
Sebastian Markbage c901b1005e Make createDescriptor return a descriptor for components
This moves all logic around legacy descriptors to ReactLegacyDescriptor. This
is responsible for the layer that knows that createClass exports a legacy
factory. When passed one of these classes, it unwraps it to be a real class.

If it is passed a non legacy factory, it is assumed to be a non-react component
that needs to be invoked as a plain function.

The semantic change is that a descriptor is now always returned if passed a
legacy factory. Even if that factory is a mock. A mock would previously return
undefined.

For mocks, I treat the factory as the authoritative function. I call it to extract
the instance or fill it with an empty component placeholder.

Additionally, I make the classes take props as the first argument to the
constructor. This is what the new class system will do.

We currently need to set up some internals by calling the internal construct
method. Instead of doing that automatically in the constructor, I now move that
to a second pass so that mocks can get the plain props.

This means that we can assert that a mock has been called once it's mounted
with it's final props. Instead of the descriptor factory being called.
2014-08-20 00:14:32 -07:00
Sebastian Markbage 3818656f70 Use Object.assign in merge/mergeInto
Relax the argument type checks. Currently we throw for non-objects and terminals
but Object.assign does a coercion to Object instead. It also allows merging
Arrays as if they are objects.

This also relaxes the check for dependents such as ImmutableObject. This sucks
but it will allow us to use a fast code path to native Object.assign.

We always have the option of adding warnings to Object.assign or static type
checks.

I'm keeping the null check. Object.assign throws for null checks.

We'll also start returning the result of coercions just like Object.assign.
2014-07-18 22:01:36 -07:00
Ben Newman 66cdba3dfb Avoid leading spaces when first argument to joinClasses is falsy.
This detail is going to become more important once the idiom
`className={joinClasses(this.props.className, newClass)}` becomes more
common, as it will when we move away from `this.transferPropsTo`.
2014-07-18 22:01:36 -07:00
Sebastian Markbage 5aab0bddaa Move key/ref off props and prepare for new descriptor factories
Breaking changes

- key/ref are no longer accessible on props but they are accessible on the
  descriptors. This means that parents/owners can access it but not the
  component itself.

- Descriptor factories are now plain functions and you can't rely on the
  prototype or constructors of descriptors to identify the component type.

Existing descriptor factories are now wrapped in a legacy factory. Currently it
does nothing but it will give us a hook to track callers to factories that are
not using JSX but just invoking the function directly. It also proxies static
methods/properties to the underlying class. The newer factories don't have this
feature.

ReactTextComponent has it's own little factory because it's props is not an
object. This is a detail and will go away once ReactTextComponent no longer
needs descriptors.
2014-07-18 22:01:36 -07:00
Ben Alpert 92d2dcc25f Revert "Add helpful message about pooled classes"
This reverts commit e65f17b86c. This might have a perf impact so we're not going to go with it for now.
2014-07-11 16:00:58 -07:00
Cheng Lou e8e08127c5 Merge pull request #1461 from spicyj/no-esc-slash
Don't escape slash; it's unnecessary
2014-07-03 11:05:16 -07:00
Paul O'Shannessy 022e44c95b Cleanup ReactChildren 2014-06-30 20:55:09 -07:00
Guangqiang Dong 1aa9cc6a8b add count() method to ReactChildren
added ReactChildren.count() to count the number of children, and a test case.
2014-06-30 20:54:57 -07:00
Cheng Lou a0486514a3 Merge pull request #1737 from spicyj/notime
Remove timing metrics from Transaction
2014-06-24 11:02:22 -07:00
Ben Alpert 1c44b874fc Remove timing metrics from Transaction
No one uses these and they weren't meant to be left in.

Test Plan: jest
2014-06-23 20:32:12 -07:00
Ben Alpert e65f17b86c Add helpful message about pooled classes
Hopefully this will make fewer people confused at all of the null properties. The string won't wrap nicely so I tried to keep it pithy:

https://s3.amazonaws.com/uploads.hipchat.com/6574/26709/v8vzKAC784QMkX2/upload.png
2014-06-23 14:30:40 -07:00
Ben Alpert 0d3622714c Don't escape slash; it's unnecessary
Fixes #1444.

Test Plan: grunt fasttest
2014-04-28 23:22:03 -03:00
Jan Kassens 514f5fb98b fix warnings in cloneWithProps test
Don't test refs in this case, already testing warning above
2014-04-23 14:14:45 -07:00
Ben Alpert f923933ef3 Make code spacing more consistent
I'm a little surprised we don't have lint rules for this.
2014-04-13 14:25:11 -07:00
Paul O’Shannessy bff6d50796 Merge pull request #1364 from spicyj/key-collisions-2
Ignore children with clashing keys
2014-04-08 18:11:36 -07:00
Ben Alpert 216fcdeb42 Ignore children with clashing keys
Fixes #566.
2014-04-07 13:50:42 -07:00
Ben Alpert ec6da04f6a Make MountReady more reusable, reduce allocations
Test Plan:
grunt test
2014-04-06 17:06:12 -07:00
Sebastian Markbage c40e06f728 First phase to true descriptors
This moves all convenience constructors to use frozen ReactDescriptors.
2014-03-28 12:32:53 -07:00
Christoph Pojer f10f32aaaf Remove objMapKeyVal 2014-03-25 13:05:44 -07:00
Kunal Mehta e505e47e01 Separate immutable project
This moves Immutable and Immutable object into the new `immutable` project.
2014-03-18 15:03:26 -07:00
Kunal Mehta 0cec4af8d7 Sync latest Immutable changes 2014-03-18 14:57:04 -07:00
Sebastian Markbage 7bbdcdba96 Reassign variable of rendered component
The component that gets passed into renderComponent isn't guaranteed to be the
instance that gets mounted. We want to clone the instance.

Unit tests need to reason about the mounted instance. The first code mod changes:

  ReactTestUtils.renderIntoDocument(<identifier>)

into

  <identifier> = ReactTestUtils.renderIntoDocument(<identifier>)

Using this scripts:

  scripts/bin/codemod -m -d ~/www --extensions js \
   '^(\s*)ReactTestUtils\.renderIntoDocument\(\s*([$a-zA-Z0-9_]+)\s*\)' \
   '\1\2 = ReactTestUtils.renderIntoDocument(\2)'

In the second case I do the same for React.renderComponent. However, there are
alot more unnecessary matches so I only codemod if the same identifier occurs
later in the file.

  scripts/bin/codemod -m -d ~/www --extensions js \
   '^(\s*)React.renderComponent\(\s*([$a-zA-Z0-9_]+)\s*?,(.*?\n?.*?\s\2\b)' \
   '\1\2 = React.renderComponent(\2,\3'

And one more for ReactMount.renderComponent used by internals.

  scripts/bin/codemod -m -d ~/www --extensions js \
   '^(\s*)ReactMount.renderComponent\(\s*([$a-zA-Z0-9_]+)\s*?,(.*?\n?.*?\s\2\b)' \
   '\1\2 = ReactMount.renderComponent(\2,\3'

This still matches many unnecessary cases where the second occurance of the
identifier is a redeclaration or comment. But this code mod doesn't hurt in
those cases.

Finally I have to do the same for:

  this.<identifier> = React.renderComponent(this.<identifier>,

This is a common pattern for production code but not tests. Some of these call
sites will likely break when we move to true descriptors.

  scripts/bin/codemod -m -d ~/www --extensions js \
   '^(\s*)React.renderComponent\((\s*)this\.([$a-zA-Z0-9\_\.]+)\s*?,' \
   '\1this.\3 = React.renderComponent(\2this.\3,'
2014-03-16 22:01:09 -07:00
Sebastian Markbage 3ea3274ca4 Clone on mount
This is the first step towards descriptors. This will start cloning the
component when it's mounted instead of mounting the first instance.

This avoids an issue where a reference to the first instance can hang around
in props. Since a mounted component gets mutated, the descriptor changes.

We don't need to clone the props object itself. Mutating the shallow props
object of a child that's passed into you is already flawed. Those cases need to
use cloneWithProps. A props object is considered shallow frozen after it leaves
the render it was created in.
2014-03-10 15:28:54 -07:00
Kunal Mehta 06f762da77 Add Immutable
As titled. First of 3 diffs to add Immutable, ImmutableMap, and ImmutableObject. No logic changes.
2014-03-04 18:14:47 -08:00
Kunal Mehta 4ad320dd13 LegacyImmutableObject module
This copies the old implementation of ImmutableObject into LegacyImmutableObject.
2014-03-04 18:14:12 -08:00
Ben Newman 41d30bb7de Re-require cloneWithProps as well, for consistency.
And don't mock `emptyObject`, since that might actually replace it with a
mutable/non-frozen object.
2014-03-04 13:12:59 -05:00
Ben Newman 9c87aef67f Fix stale usage of emptyObject in cloneWithProps-test.
After we run `require('mock-modules').dumpCache()`, the object exported by
the `emptyObject` module will no longer be identical to previously
exported objects, so tests like `expect(component.refs).toBe(emptyObject)`
will fail.

Note that this behavior only manifests itself in tests, because of course
we do not call `dumpCache` in production code.

We could consider storing the `emptyObject` globally to thwart the effects
of `dumpCache`, but it's more idiomatic simply to re-`require` the latest
version of `emptyObject`.
2014-03-04 12:59:55 -05:00
Pete Hunt 6666538316 Unbreak refs
If no refs are rendered, `this.refs` is undefined. This is bad since it deopts & is hard to look for. Instead we should make `this.refs` an immutable empty object.
2014-03-03 15:06:57 -08:00
Sebastian Markbage eee04b19e1 Add monitor module for logging instrumentation
This adds an instrumentation hook for logging so that we can monitor invalid API
usage before we're ready to issue a warning.

I took the opportunity to update some console.warns to use the warning module
instead. The remaining console.warns
will be replaced by the warning module after we've cleaned up the callsites.
2014-03-03 15:05:53 -08:00
Cheng Lou 2900997b5f Fix transaction test comments
Probably got copy/pasted in.
2014-02-20 16:54:52 -08:00
Paul O’Shannessy c0660ea6e0 Merge pull request #1100 from plievone/mapchildren-docs
Fix docs for React.Children.map, .forEach, .only.
2014-02-18 21:43:30 -08:00
Paul O’Shannessy 8a47813baa Update copyrights for 2014.
grep -rl 'Copyright 2013 Facebook' static_upstream | xargs perl -pi -w -e s/Copyright 2013 Facebook/Copyright 2013-2014 Facebook/g;'

Not going to check in a script to do this since it will just change every year.
Closes #1006
2014-02-18 17:06:43 -08:00
plievone 827c44fcd3 Fix docs for React.Children.map, .forEach, .only. 2014-02-16 22:41:30 +02:00
Paul O’Shannessy 8cf5882447 Merge pull request #1021 from spicyj/close-finally
Set isInTransaction to false even if close throws
2014-02-11 09:27:09 -08:00
Cheng Lou 9ae002503c Ensure a pooled class releases back into the pool an instance of that class
Also added tests for PooledClass.

Noticed that some places use `ReactReconcileTransaction.release(transaction)`, when `transaction` might be of another class, say `ReactServerRenderingTransaction` (still a Github PR). This catches that.
2014-02-07 13:51:14 -08:00
Pete Hunt 9730759322 Fix cloneWithProps() to allow overriding props
This is a clear bug.
2014-02-04 14:37:53 -08:00
Ben Alpert 17f602f924 Set isInTransaction to false even if close throws
If a transaction wrapper's closer throws (such as flushBatchedUpdates in ReactDefaultBatchingStrategy) then we still want to properly deinitialize the transaction by setting isInTransaction to false. Without this, we can't reuse the transaction object (in this case, all future batched updates would fail because nested transactions aren't allowed).
2014-02-04 12:03:29 -08:00
Paul O’Shannessy 19ff353fdd Merge pull request #975 from spicyj/transaction-no-catch
Refactor Transaction to not rethrow errors
2014-02-03 21:43:27 -08:00
Cheng Lou 57bf7d21f3 fix transaction comment from componentDidRender to componentDidUpdate 2014-02-03 12:52:36 -08:00
Pete Hunt 4cbc4b58f6 Support children and ref for cloneWithProps()
We're not handling these correctly.
2014-01-30 16:53:47 -08:00
Ben Alpert d300df51e1 Refactor Transaction to not rethrow errors
Rethrowing errors makes debugging harder. This makes it so that an exception in a render method can be caught using the purple stop sign (or the blue one, of course) in Chrome.
2014-01-26 15:51:37 -08:00
Paul Shen 4a5a6ad733 Transfer the key prop in cloneWithProps
`cloneWithProps` uses `ReactPropTransferer`, which ignores the `key`
prop. See https://github.com/facebook/react/pull/713

However, this is not the case with `cloneWithProps` because when someone
is cloning a component and provides a key, they mean for the clone to
take it.
2014-01-23 13:48:39 -08:00
Paul O’Shannessy 1d1a790df0 Merge pull request #901 from syranide/80chars
Fix lines longer than 80 chars
2014-01-21 17:09:24 -08:00
Andreas Svensson 559cd46181 Shortened generated "data-reactid" 2014-01-20 22:03:25 +01:00
Andreas Svensson fd02f2c1cd Fix lines longer than 80 chars 2014-01-15 15:19:14 +01:00
Pete Hunt 1e980a146f Fix bug in cloneWithProps() 2014-01-06 18:35:54 -08:00