Sourcemap misreports the locations of string literals which are JSX children

[TomasHubelbauer]

Bug Report

Current Behavior & Input Code

https://github.com/TomasHubelbauer/babel-sourcemap

import React from 'react';

export default function App() {
  return (
    <div className="App">
      <h1>Welcome to my application!</h1>
      This is my app!
      <strong>MINE.</strong>
    </div>
  )
}

In the output:

"div"
  source: src/App.jsx:5:5
  target: lib/App.js:13:39
"App"
  source: src/App.jsx:5:19
  target: lib/App.js:14:15
"h1"
  source: src/App.jsx:6:7
  target: lib/App.js:15:35
"Welcome to my application!"
  source: src/App.jsx:6:7
  target: lib/App.js:15:46
"This is my app!"
  source: src/App.jsx:5:5
  target: lib/App.js:15:77
"strong"
  source: src/App.jsx:8:7
  target: lib/App.js:15:126
"MINE."
  source: src/App.jsx:8:7
  target: lib/App.js:15:141

• div is mapped correctly
• App is mapped correctly
• h1 is mapped correctly
• Welcome to my application! is mapped to the h1
• This is my app! is mapped to the div
• strong is mapped correctly
• MINE. is mapped to the strong

So, it appears that string literals which are used as JSX children are not getting
mapped to their correct respective source locations, instead, they are mapped to the
JSX element's tag name.

Expected behavior/code

The string literals which are JSX children should be mapped to their respective
correct locations in the original source code.

Babel Configuration (babel.config.js, .babelrc, package.json#babel, cli command, .eslintrc)

• Filename: babel.config.js

const presets = [
  [
    "@babel/env",
    {
      targets: {
        edge: "17",
        firefox: "60",
        chrome: "67",
        safari: "11.1",
      },
      useBuiltIns: false
    },
  ],
  "@babel/react"
];

module.exports = { presets };

Ran using npx babel src -s true --out-dir lib.

Environment

• Babel version(s):

    "@babel/cli": "^7.7.5",
    "@babel/core": "^7.7.5",
    "@babel/polyfill": "^7.7.0",
    "@babel/preset-env": "^7.7.6",
    "@babel/preset-react": "^7.7.4"

• Node/npm version: Node 11 NPM 6.7
• OS: Windows 10
• Monorepo: ya
• How you are using Babel: through CRA which means through WebPack:
  • Sourcemap misreports the locations of string literals which are JSX children webpack/webpack#10125
  • Source map locations for React string literal children are incorrect facebook/create-react-app#8127

Possible Solution

🤷‍♂ IDK, probably some heuristic to tell from the source context we're in JSX children
needs to be added. I understand why this happens, it is just a string being passed to a
parameter of a function which is the JSX element as a whole, so I see why it gets mapped
to the JSX syntax start location. I think the way to fix this would be to add extra logic for
realizing this in the source map generation and keep track of the string literals which are
the arguments to the function and use their original locations for the string literals which
are the output arguments instead of defaulting any non-JSX child to the element's start.

[babel-bot]

Hey @TomasHubelbauer! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite."

[jridgewell]

I looked into this, and the short answer is there's no good way to fix it. The best thing it to call t.inherits() on the generated StringLiteral to map it to the original child, to at least fix the marking so that the output text maps to the start of the input text.

But that's not satisfactory for multi line texts. Because we've "cleaned" the text, all \ns will be removed and we output a single line string. That means the default printer won't see that it needs to add a line marker to the sourcemap, because the lines don't exist anymore when it's printing.

[nicolo-ribaudo]

@jridgewell Even if it's sub-optimal, maybe ensuring that the start of the string and what comes after the string have the correct mappings will be enough: you don't need to step through pieces of a multine-string when debugging, so you'll rarely notice that only the first line is mapped correctly.

[jridgewell]

Yah, that works for me. @TomasHubelbauer, are you willing to implement this?

[TomasHubelbauer]

I am! Could you please break it down for me though, I'm not able to follow what was written so far enough to know what to do. This will be my first contribution to Babel.

[jridgewell]

Essentially, the text Welcome to my application! in your example code is an AST node, called a JSXText. These AST nodes track location information, and when we print the AST nodes back into JS code, we use the attached location information to create the sourcemap. You can see this in action at ASTExplorer:

(Look for the loc field in the parsed AST.)

In order for sourcemaps to be accurate, we need to ensure the the important AST nodes contains the location information. Currently, when we transform JSX elements that contain a JSXText child, we "clean" it by removing all \n newline characters. This is performed by cleanJSXElementLiteralChild (which only operates on JSXText child nodes, despite the function's name). You can see at the bottom of that function, theres a if (str) args.push(stringLiteral(str));. What's happen here is that, if our "cleaned" JSXText contains any characters, we're pushing a new StringLiteral node into args. args is used to create to the call to React.createElement() (it forms the 3rd and any extra arguments in the call).

The StringLiteral that we create here is the important part. It's a brand new AST node. And brand new AST nodes do not contain any location tracking information (because they're generated, they're not parsed from a source file). What's happened is we called cleanJSXElementLiteralChild with a JSXText (and that JSXText contained the original location information), and we output (via args.push()) a location-less StringLiteral. We've lost location information, and that causes bad sourcemaps.

So what needs to happen is that we should preserve the location tracking information. We perform that with t.inherits. This will make our generated StringLiteral inherit the location info (and a few other things) from the original JSXText. Now when we output a sourcemap using the StringLiteral, we'll have a more accurate location for the text.

---

The code changes are:

• Import inherits from packages/babel-types/src/modifications/inherits.ts
• Call inherits with the generated StringLiteral and the original JSXText
• Update any tests that fail (you can follow https://github.com/babel/babel/blob/main/CONTRIBUTING.md#running-lintingtests, I suggest reading the OVERWRITE section)

[TomasHubelbauer]

@JLHwung I have updated my repo to show the problem still reproduces: https://github.com/TomasHubelbauer/babel-sourcemap

I have updated all dependencies and simplified the output to show the problem better.

The issue affects only string literals which are JSX/TSX children or attribute values. Tag name length and whitespace surrounding the string literal seems to play a role.

Let me know if you can still repro on your end and if you could use any assistence from me. Thank you

[liuxingbaoyu]

@TomasHubelbauer Thanks for the investigation, I'll take a look at it.

[liuxingbaoyu]

https://evanw.github.io/source-map-visualization/#NTg2ACJ1c2Ugc3RyaWN0IjsKCk9iamVjdC5kZWZpbmVQcm9wZXJ0eShleHBvcnRzLCAiX19lc01vZHVsZSIsIHsKICB2YWx1ZTogdHJ1ZQp9KTsKZXhwb3J0cy5kZWZhdWx0ID0gQXBwOwp2YXIgX3JlYWN0ID0gX2ludGVyb3BSZXF1aXJlRGVmYXVsdChyZXF1aXJlKCJyZWFjdCIpKTsKZnVuY3Rpb24gX2ludGVyb3BSZXF1aXJlRGVmYXVsdChvYmopIHsgcmV0dXJuIG9iaiAmJiBvYmouX19lc01vZHVsZSA/IG9iaiA6IHsgZGVmYXVsdDogb2JqIH07IH0KZnVuY3Rpb24gQXBwKCkgewogIGNvbnNvbGUubG9nKCd0ZXN0Jyk7CiAgcmV0dXJuIC8qI19fUFVSRV9fKi9fcmVhY3QuZGVmYXVsdC5jcmVhdGVFbGVtZW50KCJkaXYiLCB7CiAgICBjbGFzc05hbWU6ICJBcHAiCiAgfSwgLyojX19QVVJFX18qL19yZWFjdC5kZWZhdWx0LmNyZWF0ZUVsZW1lbnQoImgxIiwgbnVsbCwgIldlbGNvbWUgdG8gbXkgYXBwbGljYXRpb24hIiksICJUaGlzIGlzIG15IGFwcCEiLCAvKiNfX1BVUkVfXyovX3JlYWN0LmRlZmF1bHQuY3JlYXRlRWxlbWVudCgic3Ryb25nIiwgbnVsbCwgIk1JTkUuIikpOwp9Ci8vIyBzb3VyY2VNYXBwaW5nVVJMPUFwcC5qcy5tYXA1MzYAeyJ2ZXJzaW9uIjozLCJmaWxlIjoiQXBwLmpzIiwibmFtZXMiOlsiQXBwIiwiY29uc29sZSIsImxvZyJdLCJzb3VyY2VzIjpbIi4uL3NyYy9BcHAuanN4Il0sInNvdXJjZXNDb250ZW50IjpbImltcG9ydCBSZWFjdCBmcm9tICdyZWFjdCc7XG5cbmV4cG9ydCBkZWZhdWx0IGZ1bmN0aW9uIEFwcCgpIHtcbiAgY29uc29sZS5sb2coJ3Rlc3QnKTtcbiAgcmV0dXJuIChcbiAgICA8ZGl2IGNsYXNzTmFtZT1cIkFwcFwiPlxuICAgICAgPGgxPldlbGNvbWUgdG8gbXkgYXBwbGljYXRpb24hPC9oMT5cbiAgICAgIFRoaXMgaXMgbXkgYXBwIVxuICAgICAgPHN0cm9uZz5NSU5FLjwvc3Ryb25nPlxuICAgIDwvZGl2PlxuICApXG59XG4iXSwibWFwcGluZ3MiOiI7Ozs7OztBQUFBO0FBQTBCO0FBRVgsU0FBU0EsR0FBRyxHQUFHO0VBQzVCQyxPQUFPLENBQUNDLEdBQUcsQ0FBQyxNQUFNLENBQUM7RUFDbkIsb0JBQ0U7SUFBSyxTQUFTLEVBQUM7RUFBSyxnQkFDbEIseUNBQUksNEJBQTBCLENBQUssbUJBRW5DLDREQUFRLE9BQUssQ0FBUyxDQUNsQjtBQUVWIn0=

I don't quite get what you mean, the expected output and the actual output in the repro repo look the same.
From the visual source map, "App", Welcome to my application!, MINE. are being pointed to the correct location, while This is my app! is being pointed to the wrong location.
This seems to be expected.

[TomasHubelbauer]

Sorry I messed up the readme! I forgot to update the expected output! Please go by the notes below the expected output code block. They detail what I think is wrong in regards to the source map locations. I will check the source map visualization as soon as I can. Perhaps a problem is with my code. But why do you think This is my app! being printed in the wrong location like you say is to be expected?

[liuxingbaoyu]

#10869 (comment)
Due to technical limitations, only the first row is mapped to the correct location, which I guess is acceptable.

[TomasHubelbauer]

I see. I think the problem must be on my end now since the visualization appears to be clearly showing the mapping is correct. I guess I am translating things wrong or feeding bad input positions into the sourcemap. I hope I will be able to work around the white-space issue. Thanks for fixing this!