Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Visualize source maps in tests #15931

Merged

Conversation

nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Sep 5, 2023

Q                       A
Fixed Issues? Fixes #1, Fixes #2
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

Source maps are difficult to debug, especially because our current snapshots are useless. I built a small tool that is like source-map-visualizer, but based on text rather than interactive.

I only enabled it for one test, but I could also enable it automatically for all the tests that use source maps.

@babel-bot
Copy link
Collaborator

babel-bot commented Sep 5, 2023

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/55392/

@@ -0,0 +1,41 @@
(1:0) var t <-- (1:0) var t
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The format is (line:column) SOURCE <-- (line:column) OUTPUT

@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Sep 5, 2023

What format would you prefer?

(a) mappings point at a character

    (1:4) var t = x     <--     (1:4) var t = f
              ^                           ^

    (1:5) ar t = x      <--     (1:5) ar t = fu
              ^                           ^

(b) mappings point between two characters (note: this inserts a | in the code)

    (1:4) var |t =      <--     (1:4) var |t = 
              |                           |

    (1:5) ar t| = x     <--     (1:5) ar t| = f
              |                           |

(c) mappings point between two characters, adding · between each two characters in the code

    (1:4) v·a·r· ·t· ·=·      <--     (1:4) v·a·r· ·t· ·=· 
                 ^                                   ^

    (1:5) a·r· ·t· ·=· ·x     <--     (1:5) a·r· ·t· ·=· ·f
                 ^                                 ^

(d) mappings point between two characters, adding between each two characters in the code and replacing spaces with ·

    (1:4) v a r · t · = ·     <--     (1:4) v a r · t · = ·
                 ^                                   ^

    (1:5) a r · t · = · x     <--     (1:5) a r · t · = · f
                 ^                                 ^

The spec defines columns in mappings as follows:

The zero-based starting column of the line in the generated code that the segment represents.

@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Sep 5, 2023

Actually, I prefer this with ranges rather than just markers:

  (1:0-4) var t =                 <--    (1:0-4) var t = 
          ^^^^                                   ^^^^

  (1:4-5) var t = x               <--    (1:4-5) var t = f
              ^                                      ^

  (1:5-8) ar t = x =>             <--    (1:5-8) ar t = func
              ^^^                                    ^^^

  (1:8-8) t = x =>                <--   (1:8-17) t = function (x) 
             ><                                      ^^^^^^^^^

  (1:8-8) t = x =>                <--  (1:17-18) ion (x) {
             ><                                      ^

  (1:8-9) t = x =>                <--  (1:18-19) on (x) {
              ^                                      ^

 (1:9-13)  = x => x *             <--  (1:19-22) n (x) {
              ^^^^                                   ^^^

 (1:9-13)  = x => x *             <--    (2:2-9)   return x * 
              ^^^^                                 ^^^^^^^

(1:13-14)  => x * x               <--   (2:9-10) urn x * x
              ^                                      ^

(1:14-17) => x * x;               <--  (2:10-13) rn x * x;
              ^^^                                    ^^^

(1:17-18) x * x;                  <--  (2:13-14) x * x;
              ^                                      ^

(1:18-19)  * x;                   <--  (2:14-15)  * x;
              ^                                      ^

(1:18-19)  * x;                   <--    (3:0-2) };
              ^                                  ^^

Comment on lines +19 to +23
(91:0-39) msg: 'Welcome to Your Vue.js App' <-- (96:6-9) msg: 'W
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^

(91:0-39) msg: 'Welcome to Your Vue.js App' <-- (96:9-39) msg: 'Welcome to Your Vue.js App'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Copy link
Contributor

@armano2 armano2 Sep 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is actually really interesting, looks like source map is not correct here


not a bug in this PR tho

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, this file is all very weird. It's the result of merging the generated source map with an existing one, so the problem might be there.

@nicolo-ribaudo nicolo-ribaudo changed the title Add option to visualize source maps in tests Visualize source maps in tests Sep 8, 2023
Copy link
Member

@liuxingbaoyu liuxingbaoyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing!
Perhaps we can add visualization of identifier names in the future.

@nicolo-ribaudo nicolo-ribaudo merged commit 0effd92 into babel:main Sep 9, 2023
48 checks passed
@nicolo-ribaudo nicolo-ribaudo deleted the visualize-source-map-fixtures branch September 9, 2023 12:00
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jan 30, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: sourcemaps area: tests outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants