Lint rule to forbid access of cross-fork fields #19679
Merged
Conversation
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
We use a shared Fiber type for both reconciler forks (old and new). It is a superset of all the fields used by both forks. However, there are some fields that should only be used in the new fork, and others that should only be used in the old fork. Ideally we would enforce this with separate Flow types for each fork. The problem is that the Fiber type is accessed by some packages outside the reconciler (like React DOM), and get passed into the reconciler as arguments. So there's no way to fork the Fiber type without also forking the packages where they are used. FiberRoot has the same issue. Instead, I've added a lint rule that forbids cross-fork access of fork-specific fields. Fields that end in `_old` or `_new` are forbidden from being used inside the new or old fork respectively. Or you can specific custom fields using the ESLint plugin options. I used this plugin to find and remove references to the effect list in d2e914a.
And `subtreeTag` as new. I didn't mark `lastEffect` because that name is also used by the Hook type. Not super important; could rename to `lastEffect_old` but idk if it's worth the effort.
facebook-github-bot
added
CLA Signed
React Core Team
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 3b01f03:
|
gaearon
approved these changes
Aug 24, 2020
koto
pushed a commit
to koto/react
that referenced
this pull request
Jun 15, 2021
* Lint rule to forbid access of cross-fork fields We use a shared Fiber type for both reconciler forks (old and new). It is a superset of all the fields used by both forks. However, there are some fields that should only be used in the new fork, and others that should only be used in the old fork. Ideally we would enforce this with separate Flow types for each fork. The problem is that the Fiber type is accessed by some packages outside the reconciler (like React DOM), and get passed into the reconciler as arguments. So there's no way to fork the Fiber type without also forking the packages where they are used. FiberRoot has the same issue. Instead, I've added a lint rule that forbids cross-fork access of fork-specific fields. Fields that end in `_old` or `_new` are forbidden from being used inside the new or old fork respectively. Or you can specific custom fields using the ESLint plugin options. I used this plugin to find and remove references to the effect list in d2e914a. * Mark effect list fields as old And `subtreeTag` as new. I didn't mark `lastEffect` because that name is also used by the Hook type. Not super important; could rename to `lastEffect_old` but idk if it's worth the effort.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
We use a shared Fiber type for both reconciler forks (old and new). It is a superset of all the fields used by both forks. However, there are some fields that should only be used in the new fork, and others that should only be used in the old fork.
Ideally we would enforce this with separate Flow types for each fork. The problem is that the Fiber type is accessed by some packages outside the reconciler (like React DOM), and get passed into the reconciler as arguments. So there's no way to fork the Fiber type without also forking the packages where they are used. FiberRoot has the same issue.
Instead, I've added a lint rule that forbids cross-fork access of fork-specific fields. Fields that end in
_oldor_neware forbidden from being used inside the new or old fork respectively. Or you can specify custom fields using the ESLint plugin options.I used this plugin to find and remove references to the effect list in d2e914a.