[pull] main from facebook:main #379
Merged
+472
−0
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
@josephsavona this was briefly discussed in an old thread, lmk your thoughts on the approach. I have some fixes ready as well but wanted to get this test case in first... there's some things I don't _love_ about this approach, but end of the day it's just a tool for the test suite rather than something for end user folks so even if it does a 70% good enough job that's fine. ### refresher on the problem when we generate coverage reports with jest (istanbul), our coverage ends up completely out of whack due to the AST missing a ton of (let's call them "important") source locations after the compiler pipeline has run. At the moment to get around this, we've been doing something a bit unorthodox and also running our test suite with istanbul running before the compiler -- which results in its own set of issues (for eg, things being memoized differently, or the compiler completely bailing out on the instrumented code, etc). before getting in fixes, I wanted to set up a test case to start chipping away on as you had recommended. ### how it works The validator basically: 1. Traverses the original AST and collects the source locations for some "important" node types - (excludes useMemo/useCallback calls, as those are stripped out by the compiler) 3. Traverses the generated AST and looks for nodes with matching source locations. 4. Generates errors for source locations missing nodes in the generated AST ### caveats/drawbacks There are some things that don't work super well with this approach. A more natural test fit I think would be just having some explicit assertions made against an AST in a test file, as you can just bake all of the assumptions/nuance in there that are difficult to handle in a generic manner. However, this is maybe "good enough" for now. 1. Have to be careful what you put into the test fixture. If you put in some code that the compiler just removes (for eg, a variable assignment that is unused), you're creating a failure case that's impossible to fix. I added a skip for useMemo/useCallback. 2. "Important" locations must exactly match for validation to pass. - Might get tricky making sure things are mapped correctly when a node type is completely changed, for eg, when a block statement arrow function body gets turned into an implicit return via the body just being an expression/identifier. - This can/could result in scenarios where more changes are needed to shuttle the locations through due to HIR not having a 1:1 mapping all the babel nuances, even if some combination of other data might be good enough even if not 10000% accurate. This might be the _right_ thing anyways so we don't end up with edge cases having incorrect source locations.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )