mirror of
https://github.com/facebook/react.git
synced 2026-02-25 23:54:58 +00:00
4c3470eef8
* Harden tests around init/addition/update/removal of aliased attributes I noticed some patterns weren't being tested. * Call setValueForProperty() for null and undefined The branching before the call is unnecessary because setValueForProperty() already has an internal branch that delegates to deleteValueForProperty() for null and undefined through the shouldIgnoreValue() check. The goal is to start unifying these methods because their separation doesn't reflect the current behavior (e.g. for unknown properties) anymore, and obscures what actually happens with different inputs. * Inline deleteValueForProperty() into setValueForProperty() Now we don't read propertyInfo twice in this case. I also dropped a few early returns. I added them a while ago when we had Stack-only tracking of DOM operations, and some operations were being counted twice because of how this code is structured. This isn't a problem anymore (both because we don't track operations, and because I've just inlined this method call). * Inline deleteValueForAttribute() into setValueForAttribute() The special cases for null and undefined already exist in setValueForAttribute(). * Delete some dead code * Make setValueForAttribute() a branch of setValueForProperty() Their naming is pretty confusing by now. For example setValueForProperty() calls setValueForAttribute() when shouldSetAttribute() is false (!). I want to refactor (as in, inline and then maybe factor it out differently) the relation between them. For now, I'm consolidating the callers to use setValueForProperty(). * Make it more obvious where we skip and when we reset attributes The naming of these methods is still very vague and conflicting in some cases. Will need further work. * Rewrite setValueForProperty() with early exits This makes the flow clearer in my opinion. * Move shouldIgnoreValue() into DOMProperty It was previously duplicated. It's also suspiciously similar in purpose to shouldTreatAttributeValueAsNull() so I want to see if there is a way to unify them. * Use more specific methods for testing validity * Unify shouldTreatAttributeValueAsNull() and shouldIgnoreValue() * Remove shouldSetAttribute() Its naming was confusing and it was used all over the place instead of more specific checks. Now that we only have one call site, we might as well inline and get rid of it. * Remove unnecessary condition * Remove another unnecessary condition * Add Flow coverage * Oops * Fix lint (ESLint complains about Flow suppression) * Fix treatment of Symbol/Function values on boolean attributes They weren't being properly skipped because of the early return. I added tests for this case. * Avoid getPropertyInfo() calls I think this PR looks worse on benchmarks because we have to read propertyInfo in different places. Originally I tried to get rid of propertyInfo, but looks like it's important for performance after all. So now I'm going into the opposite direction, and precompute propertyInfo as early as possible, and then just pass it around. This way we can avoid extra lookups but keep functions nice and modular. * Pass propertyInfo as argument to getValueForProperty() It always exists because this function is only called for known properties. * Make it clearer this branch is boolean-specific I wrote this and then got confused myself. * Memoize whether propertyInfo accepts boolean value Since we run these checks for all booleans, might as well remember it. * Fix a crash when numeric property is given a Symbol * Record attribute table The changes reflect that SSR doesn't crash with symbols anymore (and just warns, consistently with the client). * Refactor attribute initialization Instead of using flags, explicitly group similar attributes/properties. * Optimization: we know built-in attributes are never invalid * Use strict comparison * Rename methods for clarity * Lint nit * Minor tweaks * Document all the different attribute types
Attribute Behavior Fixture
WIP: This is an MVP, still needs polish.
Known Issues
- There are currently two errors thrown when the page loads;
SyntaxError: missing ; before statement
Instructions
cd fixtures/attribute-behavior && yarn install && yarn start
Interpretation
Each row is an attribute which could be set on some DOM component. Some of them are invalid or mis-capitalized or mixed up versions of real ones. Each column is a value which can be passed to that attribute. Every cell has a box on the left and a box on the right. The left box shows the property (or attribute) assigned by React 15.*, and the right box shows the property (or attribute) assigned by the latest version of React 16.
Right now we use a purple outline to call out cases where the assigned property (or attribute) has changed between React 15 and 16.
This project was bootstrapped with Create React App.
You can find the guide for how to do things in a CRA here.