Sitelet https://github.com/react/react/pull/9683
Skip to content

Adding dom manipulation examples to repo - #9683

Closed
jtambe wants to merge 8492 commits into
0.3-stablefrom
master
Closed

jtambe wants to merge 8492 commits into
0.3-stablefrom
master

Conversation

@jtambe

@jtambe jtambe commented May 14, 2017

Copy link
Copy Markdown

Hi team,

I am trying to add some dom manipulation example to repo in example directory.

aickin and others added 30 commits March 26, 2017 13:32
* Only re-assign defaultValue if it is different

* Do not set value if it is the same

* Properly cover defaultValue

* Use coercion to be smart about value assignment

* Add explanation of loose type checks in value assignment.

* Add test coverage for setAttribute update.

* Only apply loose value check to text inputs

* Fix case where empty switches to zero

* Handle zero case in controlled input

* Correct mistake with default value assignment after rebase

* Do not assign bad input to number input

* Only trigger number input value attribute updates on blur

* Remove reference to LinkedValueUtils

* Record new fiber tests

* Add tests for blurred number input behavior

* Replace onBlur wrapper with rule in ChangeEventPlugin

* Sift down to only number inputs

* Re-record fiber tests

* Add test case for updating attribute on uncontrolled inputs. Make related correction

* Handle uncontrolled inputs, integrate fiber

* Reorder boolean to mitigate DOM checks

* Only assign value if it is different

* Add number input browser test fixtures

During the course of the number input fix, we uncovered many edge
cases. This commit adds browser test fixtures for each of those instances.

* Address edge case preventing number precision lower than 1 place

0.0 coerces to 0, however they are not the same value when doing
string comparision. This prevented controlled number inputs from
inputing the characters `0.00`.

Also adds test cases.

* Accommodate lack of IE9 number input support

IE9 does not support number inputs. Number inputs in IE9 fallback to
traditional text inputs. This means that accessing `input.value` will
report the raw text, rather than parsing a numeric value.

This commit makes the ReactDOMInput wrapper check to see if the `type`
prop has been configured to `"number"`. In those cases, it will
perform a comparison based upon `parseFloat` instead of the raw input
value.

* Remove footnotes about IE exponent issues

With the recent IE9 fix, IE properly inserts `e` when it produces an
invalid number.

* Address exception in IE9/10 ChangeEventPlugin blur event

On blur, inputs have their values assigned. This is so that number
inputs do not conduct unexpected behavior in
Chrome/Safari. Unfortunately, there are cases where the target
instance might be undefined in IE9/10, raising an exception.

* Migrate over ReactDOMInput.js number input fixes to Fiber

Also re-record tests

* Update number fixtures to use latest components

* Add number input test case for dashes and negative numbers

* Replace trailing dash test case with replace with dash

Also run prettier
[Fiber] Adds DEV condition around getDeclarationErrorAddendum
* Remove captured scroll feature check

IE8 was the only browser that did not support captured scroll. We no
longer have that constraint.

* Remove WINDOW_HANDLE const
If your renderer doesn't use host context, you might prefer to return null. This used to give an error:

> Invariant Violation: Expected host context to exist. This error is likely caused by a bug in React. Please file an issue.

I use a sentinel value instead now.

The code in ReactFiberHostContext is a little complicated now. We could probably also just remove the invariants.
IE11 and Edge have a password unmask field that React prevents the
display of for controlled inputs. This commit adds DOM fixture
coverage for this test case.
Since IE8 is not supported anymore, lets save those bytes!
* Move takeSnapshot from UIManager to ReactNative renderer(s)

* Prettier
IE8 was the only browser that did not support captured focus. We no
longer have that constraint.
* Show more meanignful stack trace for ReactNative errors

Clicking on the stack should jump to where the error actually occurred rather than to where it's logged in a redbox component.

* Made showDialog() return type stricter. Added mock and Flow types for ExceptionManager

* Prettier

* Handle null/string thrown errors

* Removed unused reference to emptyFunction
* Remove viewport metrics

event.pageX and event.pageY are in every browser React supports.

* Rerecord tests
I removed the scroll capture feature check for IE8, however I missed
the associated warning.
oops i broke it
sophiebits and others added 4 commits May 11, 2017 14:47
**what is the change?:**
See title.

**why make this change?:**
Both these dependencies were causing issues when working on React internally at
FB.

'lighthouse' requires node >=6 and we don't want to folks working on React to
using that version of node.
https://github.com/GoogleChrome/lighthouse/blob/master/package.json#L11

'nodegit' for me throws an error related to libssh2 and it's annoying to make
this work in every OS. See nodegit/nodegit#1266

**test plan:**
`npm/yarn install`
Runs with no errors on my CentOS machine and also on MacOSX laptop.

**issue:**
This is blocking work related to #9398
#9634)

* Fix to call deferred callbackQueue even if updates are aborted

* Add an assertion to make sure the update processed, but wasn't committed yet
@facebook-github-bot

Copy link
Copy Markdown
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact cla@fb.com if you have any questions.

xixixao and others added 22 commits May 16, 2017 15:42
* Server Rendering initial commit

Originally authored by spicyj, tweakes to rebase it and make it run by tomocchino.

Adding ReactDOMServerRendering and a createTagMarkup utility function which will be used in it.

* Fix build system, add shortcut scripts

* Make more ReactServerRendering-test unit tests pass

* Make ReactServerRendering-test pass with copious help from Ben

This is pretty hacky and I just inlined everything, but at least I sort of understand how it works now, and all of the tests are passing. There are also only 68 tests failing in the integration test suite.

* remove some unnecessary cruft

* Run prettier on ReactDOMServerRendering.js

* Fix more unit tests with Ben

* Add support for input and textarea by copy pasting a bunch of code from ReactDOMInput/ReactDOMTextarea

* Fix context unit tests with more copy paste :)

* progress on select

* Holy shit, 100% of ReactDOMServerIntegration tests are passing

* Checkpoint to fix some of the ReactDOMComponent-test tests

* Fix missing checkPropTypes

* Fix some unit tests in ReactDOMComponent-test

* Run prettier on everything, thanks Ben

* get rid of ssr tests that are looking for component file names

* add assertValidProps check

* rename flattenChildren to flattenOptionChildren per Ben

* Fix all the lint crap

* Move things around in the file and turn ReactDOMServerRenderer into a class

* remove changes I added to package.json

* remove separate createOpenTagMarkup file for now since everything should be copy pasted into a single file

* re-record Fiber tests

* Revert ReactDOMServer.js and the fiber tests per Sebastian

This also reverts the changes I made to ReactDOMComponent-test.js which removed the stack which is missing in the new server renderer"

* Rename files based on feedback

Moving src/renderers/dom/server/ReactDOMServerRendering.js to src/renderers/server/ReactServerRenderer.js and add src/renderers/server/ReactServer.js which makes this new codepath completely separate.

* Change throw to invariant, even though we probably need to remove this at some point

* fix prettier.. sigh
* Adjust some expectations of the server markup format of Fiber

Currently this case is using the stack renderer.

* Ensure debug hooks are injected into the Stack server renderer

In our tests this normally happens because ReactDOM.js injects them into
the shared module, but when Fiber is enabled or this is its own flat
bundle, that doesn't happen.

* Add package builds for new server renderer and enable tests

ReactServer -> ReactDOMServerStream

This file is going to be the replacement for ReactDOMServer.

I mock ReactDOMServer and user ReactDOMServerStream when we have
the fiber flag enabled. I'm now also enabling this as the default for
distributions builds (react-dom/server on npm and
react-dom-server.production.min.js as umd bundle).

I'm using traverseStackChildren instead of traverseAllChildren because
traverseAllChildren is now only in the isomorphic package and we don't
want to build all of that that into the server package.

I also have to require lower case react for the builds to work.
These throw because we're asserting on the warnings. Make this not throw
so that they instead only fail to assert the warnings.
The example explaining how redux's connect works doesn't use the same component name. Line 264 references (Comment) while 274 referenced (CommentList). Changed 264 to match 274.
Just keeping this fresh.

Issue:
As of Chrome 58, the Timeline tab is now called the Performance tab, this updates the "Optimizing Performance > Profiling Components with Chrome Performance" section of the docs to reflect that.
I need this for some hacky stuff I'm doing (D5060180).
* Initial regeneration of results.json

**what is the change?:**
We ran `yarn build` and updated the perf. stats record.

**why make this change?:**
Some commits have landed without updating this. By getting an initial update, I can run the build script again after my changes and see any size regressions.

* Downgrade deprecation warnings from errors to warnings

**what is the change?:**
Swapping out `warning` module for a fork that uses `console.warn`.
It looks like we were using the `warning` module for deprecation notices, *but* there is also a 'deprecated' module designed specifically for deprecation notices.

However, we could not find any place that it was currently used.

Since React's build process is not 100% clear to me, I assume it could still be used somewhere by something and just updated it along with other deprecation notices.

We might consider a follow-up diff that does some clean up here;
 - remove 'deprecated' module if it's unused, OR
 - use 'deprecated' module for all our current deprecation warnings

**why make this change?:**
- We have had complaints about noisy warnings, in particular after introducing new deprecations
- They potentially cause CI failures
- Deprecations are not really time-sensitive, can ship without breaking your app, etc.

For more context - #9395

**test plan:**
`npm run test`
and unit tests for the new modules
and manual testing (WIP)

**issue:**
#9395

* Add 'lowPriorityWarning' to ReactExternals

**what is the change?:**
We won't bundle 'lowPriorityWarning' with the rest of React when building for Facebook.
NOTE: A parallel commit will introduce an internal implementation of 'lowPriorityWarning' in Facebook's codebase, to compensate. Will post a comment with the diff number once that is up.

**why make this change?:**
So that the sync between github and Facebook can go more smoothly!

**test plan:**
We will see when I run the sync! But this is a reasonable first step imo.

**issue:**
#9398

* Make state mutations an error, not low-pri warning

**what is the change?:**
Even though this is a "deprecation" warning, we still want to use 'console.error' for it.

**why make this change?:**
- It's not likely to come up now, hopefully, because this warning has been present for some time
- This will cause real issues in production if ignored

**test plan:**
`yarn test` - we did fix one test which failed bc of this change

**issue:**
#9398

* Fix test of assigning to this.state that was only passing in fiber

**what is the change?:**
updated a unit test for assigning directly to state; it once again raises an error and not a warning.

**why make this change?:**
So that tests pass

**test plan:**
 REACT_DOM_JEST_USE_FIBER=1 yarn run test

**issue:**

* Update results.json
* fixed conflicts with master

* splits fiber and stack implementations

* prettier run

* updated fiber implementation based on feedback

* updated fiber implementation for selecting inspector hierarchy

* run of prettier

* fixed flow issues

* updated stack implementation

* fixes an implementation difference where before it wasnt properly understoof

* addresses comments in PR feedback

* updated stack inspector to use emptyObject

* Update ReactNativeFiberInspector.js

Fixes a flow error

* fixes last flow error

* prettier

* applied changes to how viewConfig works for fibers and extracted measure out

* fixed bad paste

* fixes flow errors

* prettyify

* revmoed getComponentName changes

* updated logic for getHostProps and addressed other PR feedback

* improved throwing to invariant and update stack implemenation based on feedback

* prettier
Just two more items before we will have an RC ready. :)
* Add back caught error and other checks to 'lowPriorityWarning'

**what is the change?:**
This change makes 'lowPriorityWarning' an exact copy of 'warning.js' from
https://github.com/facebook/fbjs/blob/e66ba20ad5be433eb54423f2b097d829324d9de6/packages/fbjs/src/__forks__/warning.js
where before we had skipped some checks from that module.

- Adds an error which we catch, in order to let people find the error and resulting stack trace when using devtools with 'pause on caught errors' checked.
- Adds check that 'format' argument is passed

**why make this change?:**
- To maintain a closer fork to 'warning.js'
- To allow easier debugging using 'pause on caught errors'
- To validate inputs to 'lowPriorityWarning'

**test plan:**
`yarn test`

**issue:**

* Update results.json

* Run prettier
* Split ReactNativeFiber into separate ReactNativeFiberRenderer module
Hopefully this is sufficient to work around Rollup circular dependency problems. (To be seen in subsequent commits...)

* Split findNodeHandle into findNodeHandleFiber + findNodeHandleStack
This allowed me to remove the ReactNative -> findNodeHandle injections, which should in turn allow me to require a fully-functional findNodeHandle without going through ReactNative. This will hopefully allow ReactNativeBaseomponent to avoid a circular dependency.

* Un-forked findNodeHandle in favor of just inlining the findNode function impl

* takeSnapshot no longer requires/depends-on ReactNative for findNodeHandle
Instead it uses the new, renderer-specific wrappers (eg findNodeHandleFiberWrapper and findNodeHandleStackWrapper) to ensure the returned value is numeric (or null). This avoids a circular dependency that would trip up Rollup.

* NativeMethodsMixin requires findNodeHandler wrapper(s) directly rather than ReactNative
This works around a potential circular dependency that would break the Rollup build

* Add RN_* build targets to hash-finle-name check

* Strip @providesModule annotations from headers for RN_* builds

* Added process.env.REACT_NATIVE_USE_FIBER to ReactNativeFeatureFlags
This is kind of a hacky solution, but it is temporary. It works around the fact that ReactNativeFeatureFlag values need to be set at build time in order to avoid a mismatch between runtime flag values. DOM avoids the need to do this by using injection but Native is not able to use this same approach due to circular dependency issues.

* Moved a couple of SECRET exports to dev-only. Removed SyntheticEvent and PooledClass from SECRET exports. Converted Rollup helper function to use named params.

* Split NativeMethodsMixins interface and object-type

* Add @noflow header to flat-bundle template to avoid triggering Flow problems
When Flow tries to infer such a large file, it consumes massive amounts of CPU/RAM and can often lead to programs crashing. It is better for such large files to use .flow.js types instead.

* NativeMethodsMixin and ReactNativeFiberHostComponent now share the same Flow type

* Collocated (externally exposed) ReactTypes and ReactNativeTypes into single files to be synced to fbsource. ReactNativeFiber and ReactNativeStack use ReactNativeType Flow type

* Build script syncs RN types and PooledClass automatically

* Added optional sync-RN step to Rollup build script

* Added results.json for new RN bundles
@gaearon

gaearon commented May 24, 2017

Copy link
Copy Markdown
Contributor

I'm confused by this PR, it seems botched. If you'd like to send a new one with your proposed change please do.

@gaearon gaearon closed this May 24, 2017
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.