[compiler] Null out source locations where not explicitly preserved #33051
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
Inspired by #32950. Specifically from #32950 (comment), it sounds like Babel by default emits source map information for all nodes, even when they don't have a `loc` property set. Code coverage tools then pick up the synthesized source location information for this, leading to the issue described. A few google searches didn't turn up any documented way to opt-out of generating source span information, but AI tools answer that explicitly setting `node.loc = null` omits the node from source maps. This PR is a quick hack to confirm that. [ghstack-poisoned]
josephsavona
added a commit
that referenced
this pull request
Apr 29, 2025
Inspired by #32950. Specifically from #32950 (comment), it sounds like Babel by default emits source map information for all nodes, even when they don't have a `loc` property set. Code coverage tools then pick up the synthesized source location information for this, leading to the issue described. A few google searches didn't turn up any documented way to opt-out of generating source span information, but AI tools answer that explicitly setting `node.loc = null` omits the node from source maps. This PR is a quick hack to confirm that. ghstack-source-id: 8ec8b35815c2c798c6850ffda1661916e3208110 Pull Request resolved: #33051
|
edit: addressed, see updated description |
…preserved" Inspired by #32950. Specifically from #32950 (comment), it sounds like Babel by default emits source map information for all nodes, even when they don't have a `loc` property set. Code coverage tools then pick up the synthesized source location information for this, leading to the issue described. A few google searches didn't turn up any documented way to opt-out of generating source span information, but AI tools answer that explicitly setting `node.loc = null` omits the node from source maps. This PR is a quick hack to confirm that. [ghstack-poisoned]
josephsavona
added a commit
that referenced
this pull request
Apr 30, 2025
Inspired by #32950. Specifically from #32950 (comment), it sounds like Babel by default emits source map information for all nodes, even when they don't have a `loc` property set. Code coverage tools then pick up the synthesized source location information for this, leading to the issue described. A few google searches didn't turn up any documented way to opt-out of generating source span information, but AI tools answer that explicitly setting `node.loc = null` omits the node from source maps. This PR is a quick hack to confirm that. ghstack-source-id: f089cddb4b5f35050491fa1c02a4b1484f757797 Pull Request resolved: #33051
josephsavona
commented
Apr 30, 2025
Comment on lines
+2683
to
+2688
| ast['loc'] = { | ||
| start: {line: null, column: null, index: null}, | ||
| end: {line: null, column: null, index: null}, | ||
| filename: null, | ||
| identifierName: null, | ||
| }; |
There was a problem hiding this comment.
At least on the version of Babel we're using for tests, just setting loc: null wasn't enough to exclude the node from source maps. But setting all of this was enough to exclude them.
…preserved" Inspired by #32950. Specifically from #32950 (comment), it sounds like Babel by default emits source map information for all nodes, even when they don't have a `loc` property set. Code coverage tools then pick up the synthesized source location information for this, leading to the issue described. A few google searches didn't turn up any documented way to opt-out of generating source span information, but AI tools answer that explicitly setting `node.loc = null` omits the node from source maps. This PR is a quick hack to confirm that. [ghstack-poisoned]
josephsavona
added a commit
that referenced
this pull request
Apr 30, 2025
Inspired by #32950. Specifically from #32950 (comment), it sounds like Babel by default emits source map information for all nodes, even when they don't have a `loc` property set. Code coverage tools then pick up the synthesized source location information for this, leading to the issue described. A few google searches didn't turn up any documented way to opt-out of generating source span information, but AI tools answer that explicitly setting `node.loc = null` omits the node from source maps. This PR is a quick hack to confirm that. ghstack-source-id: 3f2255dfbec8315994d034c45e16802ad7314a86 Pull Request resolved: #33051
…preserved" Inspired by #32950. Specifically from #32950 (comment), it sounds like Babel by default emits source map information for all nodes, even when they don't have a `loc` property set. Code coverage tools then pick up the synthesized source location information for this, leading to the issue described. It wasn't super clear how to omit nodes from the source map. A few google searches didn't turn up any documented way to do so: the user can do so via a function supplied to babel, but plugins can't. Entire files can be opted out, but not nodes. I asked an LLM which answered that explicitly setting `node.loc = null` omits the node from source maps, but that didn't work. What did work was explicitly setting the `loc` property to an object that follows the shape of a source location, but with all properties nulled out. With that, we get the desired result: <img width="553" alt="Screenshot 2025-04-30 at 10 50 30 AM" src="https://github.com/user-attachments/assets/cf55fd32-8375-4cfa-8202-49b45ef02931" /> I'm going to feature-flag this since setting some of these properties in this way might break in other versions of Babel or have other issues. [ghstack-poisoned]
mofeiZ
approved these changes
Apr 30, 2025
There was a problem hiding this comment.
Makes sense, I wonder how this affects setting debugger breakpoints with source maps enabled -- missing / greyed out breakpoints are probably less confusing than misleading ones, so let's try it
On a side note, hopefully this is a no-op for fbt enums which rely on sourcemaps
…preserved" Inspired by #32950. Specifically from #32950 (comment), it sounds like Babel by default emits source map information for all nodes, even when they don't have a `loc` property set. Code coverage tools then pick up the synthesized source location information for this, leading to the issue described. It wasn't super clear how to omit nodes from the source map. A few google searches didn't turn up any documented way to do so: the user can do so via a function supplied to babel, but plugins can't. Entire files can be opted out, but not nodes. I asked an LLM which answered that explicitly setting `node.loc = null` omits the node from source maps, but that didn't work. What did work was explicitly setting the `loc` property to an object that follows the shape of a source location, but with all properties nulled out. With that, we get the desired result: <img width="553" alt="Screenshot 2025-04-30 at 10 50 30 AM" src="https://github.com/user-attachments/assets/cf55fd32-8375-4cfa-8202-49b45ef02931" /> I'm going to feature-flag this since setting some of these properties in this way might break in other versions of Babel or have other issues. [ghstack-poisoned]
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.
Stack from ghstack (oldest at bottom):
Inspired by #32950. Specifically from #32950 (comment), it sounds like Babel by default emits source map information for all nodes, even when they don't have a
locproperty set. Code coverage tools then pick up the synthesized source location information for this, leading to the issue described.It wasn't super clear how to omit nodes from the source map. A few google searches didn't turn up any documented way to do so: the user can do so via a function supplied to babel, but plugins can't. Entire files can be opted out, but not nodes. I asked an LLM which answered that explicitly setting
node.loc = nullomits the node from source maps, but that didn't work.What did work was explicitly setting the
locproperty to an object that follows the shape of a source location, but with all properties nulled out. With that, we get the desired result:I'm going to feature-flag this since setting some of these properties in this way might break in other versions of Babel or have other issues.