Conversation
* 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
…m/giamir/react into giamir-add_toggle_to_fiber_triangle_demo
[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.
D4616127 D4629747 D4631778
CI still checks all of them.
**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
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. |
* 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.
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
* 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
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi team,
I am trying to add some dom manipulation example to repo in example directory.