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

Add experimental format-preserving mode to @babel/generator #16708

Merged
merged 73 commits into from
Oct 23, 2024

Conversation

nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Aug 1, 2024

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

This PR implements a experimental_preserveFormat: true option for @babel/generator that lets it print code fully respecting its original format.

This is done by parsing the code with tokens: true and by passing the original source code to @babel/generator: whenever it needs to print a token, it tries to match it with one of the tokens in the input file and prints it in the same location, if possible. When there are new tokens (for new nodes), it prints them as compact as possible so that the next token has a chance of being printed in the correct position, rather than being "pushed ahead".

This option currently only works with retainLines: true. If throws an error if you don't explicitly set it, because in the future I plan to investigate removing that restriction (so that, for example, a codemod could inject new nodes in the middle and the rest gets shifted without being re-formatted).

This mode works for JS (including proposals), JSX and TypeScript. Unfortunately our Flow AST makes it too difficult to apply the same strategy there.

packages/babel-generator/test/preserve-format.js is verifying that we are able to parse and re-print all the existing generator tests and get exactly the same output. There is a list of failures -- I divided them in a few groups, and they are all expected failures because they are testing some cases in which the options explicitly ask to re-print the code in a different way.

A lot of changes have been extracted to separate PRs, at https://github.com/babel/babel/milestone/61. There are still a couple PRs that this one depends on (I currently just cherry-picked them), so go review them first! :)

@nicolo-ribaudo nicolo-ribaudo added PR: New Feature 🚀 A type of pull request used for our changelog categories pkg: generator labels Aug 1, 2024
@babel-bot
Copy link
Collaborator

babel-bot commented Aug 1, 2024

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

@nicolo-ribaudo nicolo-ribaudo force-pushed the wip-preserve-format branch 2 times, most recently from df52872 to 0d14021 Compare August 22, 2024 10:44
@@ -74,6 +74,6 @@
"sourcesContent": [
"// From packages\\babel-cli\\src\\babel\\watcher.ts\n\nimport { createRequire } from \"module\";\nimport path from \"path\";\nimport type { WatchOptions, FSWatcher } from \"chokidar\";\n\nconst fileToDeps = new Map<string, Set<string>>();\nconst depToFiles = new Map<string, Set<string>>();\n\nlet isWatchMode = false;\nlet watcher: FSWatcher;\nconst watchQueue = new Set<string>();\nlet hasStarted = false;\n\nexport function enable({ enableGlobbing }: { enableGlobbing: boolean }) {\n isWatchMode = true;\n\n const { FSWatcher } = requireChokidar();\n\n const options: WatchOptions = {\n disableGlobbing: !enableGlobbing,\n persistent: true,\n ignoreInitial: true,\n awaitWriteFinish: {\n stabilityThreshold: 50,\n pollInterval: 10,\n },\n };\n watcher = new FSWatcher(options);\n\n watcher.on(\"unlink\", unwatchFile);\n}\n\nexport function startWatcher() {\n hasStarted = true;\n\n for (const dep of watchQueue) {\n watcher.add(dep);\n }\n watchQueue.clear();\n\n watcher.on(\"ready\", () => {\n console.log(\"The watcher is ready.\");\n });\n}\n\nexport function watch(filename: string): void {\n if (!isWatchMode) {\n throw new Error(\n \"Internal Babel error: .watch called when not in watch mode.\",\n );\n }\n\n if (!hasStarted) {\n watchQueue.add(path.resolve(filename));\n } else {\n watcher.add(path.resolve(filename));\n }\n}\n\n/**\n * Call @param callback whenever a dependency (source file)/\n * external dependency (non-source file) changes.\n *\n * Handles mapping external dependencies to their corresponding\n * dependencies.\n */\nexport function onFilesChange(\n callback: (filenames: string[], event: string, cause: string) => void,\n): void {\n if (!isWatchMode) {\n throw new Error(\n \"Internal Babel error: .onFilesChange called when not in watch mode.\",\n );\n }\n\n watcher.on(\"all\", (event, filename) => {\n if (event !== \"change\" && event !== \"add\") return;\n\n const absoluteFile = path.resolve(filename);\n callback(\n [absoluteFile, ...(depToFiles.get(absoluteFile) ?? [])],\n event,\n absoluteFile,\n );\n });\n}\n\nexport function updateExternalDependencies(\n filename: string,\n dependencies: Set<string>,\n) {\n if (!isWatchMode) return;\n\n // Use absolute paths\n const absFilename = path.resolve(filename);\n const absDependencies = new Set(\n Array.from(dependencies, dep => path.resolve(dep)),\n );\n\n const deps = fileToDeps.get(absFilename);\n if (deps) {\n for (const dep of deps) {\n if (!absDependencies.has(dep)) {\n removeFileDependency(absFilename, dep);\n }\n }\n }\n for (const dep of absDependencies) {\n let deps = depToFiles.get(dep);\n if (!deps) {\n depToFiles.set(dep, (deps = new Set()));\n\n if (!hasStarted) {\n watchQueue.add(dep);\n } else {\n watcher.add(dep);\n }\n }\n\n deps.add(absFilename);\n }\n\n fileToDeps.set(absFilename, absDependencies);\n}\n\nfunction removeFileDependency(filename: string, dep: string) {\n const deps = depToFiles.get(dep) as Set<string>;\n deps.delete(filename);\n\n if (deps.size === 0) {\n depToFiles.delete(dep);\n\n if (!hasStarted) {\n watchQueue.delete(dep);\n } else {\n watcher.unwatch(dep);\n }\n }\n}\n\nfunction unwatchFile(filename: string) {\n const deps = fileToDeps.get(filename);\n if (!deps) return;\n\n for (const dep of deps) {\n removeFileDependency(filename, dep);\n }\n fileToDeps.delete(filename);\n}\n\nfunction requireChokidar(): any {\n const require = createRequire(import.meta.url);\n\n try {\n return process.env.BABEL_8_BREAKING\n ? require(\"chokidar\")\n : parseInt(process.versions.node) >= 8\n ? require(\"chokidar\")\n : require(\"@nicolo-ribaudo/chokidar-2\");\n } catch (err) {\n console.error(\n \"The optional dependency chokidar failed to install and is required for \" +\n \"--watch. Chokidar is likely not supported on your platform.\",\n );\n throw err;\n }\n}"
],
"mappings": "AAAA;;AAEA,SAASA,aAAa,QAAQ,QAAQ;AACtC,OAAOC,IAAI,MAAM,MAAM;AACvB,cAAcC,YAAY,EAAEC,SAAS,QAAQ,UAAU;AAEvD,MAAMC,UAAU,GAAG,IAAIC,GAAG,CAAC,MAAM,EAAEC,GAAG,CAAC,MAAM,CAAC,CAAC,CAAC,CAAC;AACjD,MAAMC,UAAU,GAAG,IAAIF,GAAG,CAAC,MAAM,EAAEC,GAAG,CAAC,MAAM,CAAC,CAAC,CAAC,CAAC;AAEjD,IAAIE,WAAW,GAAG,KAAK;AACvB,IAAIC,OAAO,EAAEN,SAAS;AACtB,MAAMO,UAAU,GAAG,IAAIJ,GAAG,CAAC,MAAM,CAAC,CAAC,CAAC;AACpC,IAAIK,UAAU,GAAG,KAAK;AAEtB,OAAO,SAASC,MAAMA,CAAC;EAAEC;AAA4C,CAA5B,EAAE;EAAEA,cAAc,EAAE,OAAO;AAAC,CAAC,EAAE;EACtEL,WAAW,GAAG,IAAI;EAElB,MAAM;IAAEL;EAAU,CAAC,GAAGW,eAAe,CAAC,CAAC;EAEvC,MAAMC,OAAO,EAAEb,YAAY,GAAG;IAC5Bc,eAAe,EAAE,CAACH,cAAc;IAChCI,UAAU,EAAE,IAAI;IAChBC,aAAa,EAAE,IAAI;IACnBC,gBAAgB,EAAE;MAChBC,kBAAkB,EAAE,EAAE;MACtBC,YAAY,EAAE;IAChB;EACF,CAAC;EACDZ,OAAO,GAAG,IAAIN,SAAS,CAACY,OAAO,CAAC;EAEhCN,OAAO,CAACa,EAAE,CAAC,QAAQ,EAAEC,WAAW,CAAC;AACnC;AAEA,OAAO,SAASC,YAAYA,CAAA,EAAG;EAC7Bb,UAAU,GAAG,IAAI;EAEjB,KAAK,MAAMc,GAAG,IAAIf,UAAU,EAAE;IAC5BD,OAAO,CAACiB,GAAG,CAACD,GAAG,CAAC;EAClB;EACAf,UAAU,CAACiB,KAAK,CAAC,CAAC;EAElBlB,OAAO,CAACa,EAAE,CAAC,OAAO,EAAE,MAAM;IACxBM,OAAO,CAACC,GAAG,CAAC,uBAAuB,CAAC;EACtC,CAAC,CAAC;AACJ;AAEA,OAAO,SAASC,KAAKA,CAACC,QAAQ,EAAE,MAAM,CAAC,EAAE,IAAI,CAAC;EAC5C,IAAI,CAACvB,WAAW,EAAE;IAChB,MAAM,IAAIwB,KAAK,CACb,6DACF,CAAC;EACH;EAEA,IAAI,CAACrB,UAAU,EAAE;IACfD,UAAU,CAACgB,GAAG,CAACzB,IAAI,CAACgC,OAAO,CAACF,QAAQ,CAAC,CAAC;EACxC,CAAC,MAAM;IACLtB,OAAO,CAACiB,GAAG,CAACzB,IAAI,CAACgC,OAAO,CAACF,QAAQ,CAAC,CAAC;EACrC;AACF;;AAEA;AACA;AACA;AACA;AACA;AACA;AACA;AACA,OAAO,SAASG,aAAaA,CAC3BC,QAAQ,EAAE,CAACC,SAAS,EAAE,MAAM,EAAE,EAAEC,KAAK,EAAE,MAAM,EAAEC,KAAK,EAAE,MAAM,KAAK,IAAI,CACtE,EAAE,IAAI,CAAC;EACN,IAAI,CAAC9B,WAAW,EAAE;IAChB,MAAM,IAAIwB,KAAK,CACb,qEACF,CAAC;EACH;EAEAvB,OAAO,CAACa,EAAE,CAAC,KAAK,EAAE,CAACe,KAAK,EAAEN,QAAQ,KAAK;IACrC,IAAIM,KAAK,KAAK,QAAQ,IAAIA,KAAK,KAAK,KAAK,EAAE;IAE3C,MAAME,YAAY,GAAGtC,IAAI,CAACgC,OAAO,CAACF,QAAQ,CAAC;IAC3CI,QAAQ,CACN,CAACI,YAAY,EAAE,IAAIhC,UAAU,CAACiC,GAAG,CAACD,YAAY,CAAC,IAAI,EAAE,CAAC,CAAC,EACvDF,KAAK,EACLE,YACF,CAAC;EACH,CAAC,CAAC;AACJ;AAEA,OAAO,SAASE,0BAA0BA,CACxCV,QAAQ,EAAE,MAAM,EAChBW,YAAY,EAAEpC,GAAG,CAAC,MAAM,CAAC,EACzB;EACA,IAAI,CAACE,WAAW,EAAE;;EAElB;EACA,MAAMmC,WAAW,GAAG1C,IAAI,CAACgC,OAAO,CAACF,QAAQ,CAAC;EAC1C,MAAMa,eAAe,GAAG,IAAItC,GAAG,CAC7BuC,KAAK,CAACC,IAAI,CAACJ,YAAY,EAAEjB,GAAG,IAAIxB,IAAI,CAACgC,OAAO,CAACR,GAAG,CAAC,CACnD,CAAC;EAED,MAAMsB,IAAI,GAAG3C,UAAU,CAACoC,GAAG,CAACG,WAAW,CAAC;EACxC,IAAII,IAAI,EAAE;IACR,KAAK,MAAMtB,GAAG,IAAIsB,IAAI,EAAE;MACtB,IAAI,CAACH,eAAe,CAACI,GAAG,CAACvB,GAAG,CAAC,EAAE;QAC7BwB,oBAAoB,CAACN,WAAW,EAAElB,GAAG,CAAC;MACxC;IACF;EACF;EACA,KAAK,MAAMA,GAAG,IAAImB,eAAe,EAAE;IACjC,IAAIG,IAAI,GAAGxC,UAAU,CAACiC,GAAG,CAACf,GAAG,CAAC;IAC9B,IAAI,CAACsB,IAAI,EAAE;MACTxC,UAAU,CAAC2C,GAAG,CAACzB,GAAG,EAAGsB,IAAI,GAAG,IAAIzC,GAAG,CAAC,CAAE,CAAC;MAEvC,IAAI,CAACK,UAAU,EAAE;QACfD,UAAU,CAACgB,GAAG,CAACD,GAAG,CAAC;MACrB,CAAC,MAAM;QACLhB,OAAO,CAACiB,GAAG,CAACD,GAAG,CAAC;MAClB;IACF;IAEAsB,IAAI,CAACrB,GAAG,CAACiB,WAAW,CAAC;EACvB;EAEAvC,UAAU,CAAC8C,GAAG,CAACP,WAAW,EAAEC,eAAe,CAAC;AAC9C;AAEA,SAASK,oBAAoBA,CAAClB,QAAQ,EAAE,MAAM,EAAEN,GAAG,EAAE,MAAM,EAAE;EAC3D,MAAMsB,IAAI,GAAGxC,UAAU,CAACiC,GAAG,CAACf,GAAG,CAAC,IAAInB,GAAG,CAAC,MAAM,CAAC;EAC/CyC,IAAI,CAACI,MAAM,CAACpB,QAAQ,CAAC;EAErB,IAAIgB,IAAI,CAACK,IAAI,KAAK,CAAC,EAAE;IACnB7C,UAAU,CAAC4C,MAAM,CAAC1B,GAAG,CAAC;IAEtB,IAAI,CAACd,UAAU,EAAE;MACfD,UAAU,CAACyC,MAAM,CAAC1B,GAAG,CAAC;IACxB,CAAC,MAAM;MACLhB,OAAO,CAAC4C,OAAO,CAAC5B,GAAG,CAAC;IACtB;EACF;AACF;AAEA,SAASF,WAAWA,CAACQ,QAAQ,EAAE,MAAM,EAAE;EACrC,MAAMgB,IAAI,GAAG3C,UAAU,CAACoC,GAAG,CAACT,QAAQ,CAAC;EACrC,IAAI,CAACgB,IAAI,EAAE;EAEX,KAAK,MAAMtB,GAAG,IAAIsB,IAAI,EAAE;IACtBE,oBAAoB,CAAClB,QAAQ,EAAEN,GAAG,CAAC;EACrC;EACArB,UAAU,CAAC+C,MAAM,CAACpB,QAAQ,CAAC;AAC7B;AAEA,SAASjB,eAAeA,CAAA,CAAE,EAAE,GAAG,CAAC;EAC9B,MAAMwC,OAAO,GAAGtD,aAAa,CAACuD,MAAM,CAACC,IAAI,CAACC,GAAG,CAAC;EAE9C,IAAI;IACF,OAAOC,OAAO,CAACC,GAAG,CAACC,gBAAgB,GAC/BN,OAAO,CAAC,UAAU,CAAC,GACnBO,QAAQ,CAACH,OAAO,CAACI,QAAQ,CAACC,IAAI,CAAC,IAAI,CAAC,GACpCT,OAAO,CAAC,UAAU,CAAC,GACnBA,OAAO,CAAC,4BAA4B,CAAC;EAC3C,CAAC,CAAC,OAAOU,GAAG,EAAE;IACZpC,OAAO,CAACqC,KAAK,CACX,yEAAyE,GACvE,6DACJ,CAAC;IACD,MAAMD,GAAG;EACX;AACF",
"mappings": "AAAA;;AAEA,SAASA,aAAa,QAAQ,QAAQ;AACtC,OAAOC,IAAI,MAAM,MAAM;AACvB,cAAcC,YAAY,EAAEC,SAAS,QAAQ,UAAU;AAEvD,MAAMC,UAAU,GAAG,IAAIC,GAAG,CAAC,MAAM,EAAEC,GAAG,CAAC,MAAM,CAAC,CAAC,CAAC,CAAC;AACjD,MAAMC,UAAU,GAAG,IAAIF,GAAG,CAAC,MAAM,EAAEC,GAAG,CAAC,MAAM,CAAC,CAAC,CAAC,CAAC;AAEjD,IAAIE,WAAW,GAAG,KAAK;AACvB,IAAIC,OAAO,EAAEN,SAAS;AACtB,MAAMO,UAAU,GAAG,IAAIJ,GAAG,CAAC,MAAM,CAAC,CAAC,CAAC;AACpC,IAAIK,UAAU,GAAG,KAAK;AAEtB,OAAO,SAASC,MAAMA,CAAC;EAAEC;AAA4C,CAA5B,EAAE;EAAEA,cAAc,EAAE,OAAO;AAAC,CAAC,EAAE;EACtEL,WAAW,GAAG,IAAI;EAElB,MAAM;IAAEL;EAAU,CAAC,GAAGW,eAAe,CAAC,CAAC;EAEvC,MAAMC,OAAO,EAAEb,YAAY,GAAG;IAC5Bc,eAAe,EAAE,CAACH,cAAc;IAChCI,UAAU,EAAE,IAAI;IAChBC,aAAa,EAAE,IAAI;IACnBC,gBAAgB,EAAE;MAChBC,kBAAkB,EAAE,EAAE;MACtBC,YAAY,EAAE;IAChB;EACF,CAAC;EACDZ,OAAO,GAAG,IAAIN,SAAS,CAACY,OAAO,CAAC;EAEhCN,OAAO,CAACa,EAAE,CAAC,QAAQ,EAAEC,WAAW,CAAC;AACnC;AAEA,OAAO,SAASC,YAAYA,CAAA,EAAG;EAC7Bb,UAAU,GAAG,IAAI;EAEjB,KAAK,MAAMc,GAAG,IAAIf,UAAU,EAAE;IAC5BD,OAAO,CAACiB,GAAG,CAACD,GAAG,CAAC;EAClB;EACAf,UAAU,CAACiB,KAAK,CAAC,CAAC;EAElBlB,OAAO,CAACa,EAAE,CAAC,OAAO,EAAE,MAAM;IACxBM,OAAO,CAACC,GAAG,CAAC,uBAAuB,CAAC;EACtC,CAAC,CAAC;AACJ;AAEA,OAAO,SAASC,KAAKA,CAACC,QAAQ,EAAE,MAAM,CAAC,EAAE,IAAI,CAAC;EAC5C,IAAI,CAACvB,WAAW,EAAE;IAChB,MAAM,IAAIwB,KAAK,CACb,6DACF,CAAC;EACH;EAEA,IAAI,CAACrB,UAAU,EAAE;IACfD,UAAU,CAACgB,GAAG,CAACzB,IAAI,CAACgC,OAAO,CAACF,QAAQ,CAAC,CAAC;EACxC,CAAC,MAAM;IACLtB,OAAO,CAACiB,GAAG,CAACzB,IAAI,CAACgC,OAAO,CAACF,QAAQ,CAAC,CAAC;EACrC;AACF;;AAEA;AACA;AACA;AACA;AACA;AACA;AACA;AACA,OAAO,SAASG,aAAaA,CAC3BC,QAAQ,EAAE,CAACC,SAAS,EAAE,MAAM,EAAE,EAAEC,KAAK,EAAE,MAAM,EAAEC,KAAK,EAAE,MAAM,EAAE,GAAG,IAAI,CACtE,EAAE,IAAI,CAAC;EACN,IAAI,CAAC9B,WAAW,EAAE;IAChB,MAAM,IAAIwB,KAAK,CACb,qEACF,CAAC;EACH;EAEAvB,OAAO,CAACa,EAAE,CAAC,KAAK,EAAE,CAACe,KAAK,EAAEN,QAAQ,KAAK;IACrC,IAAIM,KAAK,KAAK,QAAQ,IAAIA,KAAK,KAAK,KAAK,EAAE;IAE3C,MAAME,YAAY,GAAGtC,IAAI,CAACgC,OAAO,CAACF,QAAQ,CAAC;IAC3CI,QAAQ,CACN,CAACI,YAAY,EAAE,IAAIhC,UAAU,CAACiC,GAAG,CAACD,YAAY,CAAC,IAAI,EAAE,CAAC,CAAC,EACvDF,KAAK,EACLE,YACF,CAAC;EACH,CAAC,CAAC;AACJ;AAEA,OAAO,SAASE,0BAA0BA,CACxCV,QAAQ,EAAE,MAAM,EAChBW,YAAY,EAAEpC,GAAG,CAAC,MAAM,CAAC,EACzB;EACA,IAAI,CAACE,WAAW,EAAE;;EAElB;EACA,MAAMmC,WAAW,GAAG1C,IAAI,CAACgC,OAAO,CAACF,QAAQ,CAAC;EAC1C,MAAMa,eAAe,GAAG,IAAItC,GAAG,CAC7BuC,KAAK,CAACC,IAAI,CAACJ,YAAY,EAAEjB,GAAG,IAAIxB,IAAI,CAACgC,OAAO,CAACR,GAAG,CAAC,CACnD,CAAC;EAED,MAAMsB,IAAI,GAAG3C,UAAU,CAACoC,GAAG,CAACG,WAAW,CAAC;EACxC,IAAII,IAAI,EAAE;IACR,KAAK,MAAMtB,GAAG,IAAIsB,IAAI,EAAE;MACtB,IAAI,CAACH,eAAe,CAACI,GAAG,CAACvB,GAAG,CAAC,EAAE;QAC7BwB,oBAAoB,CAACN,WAAW,EAAElB,GAAG,CAAC;MACxC;IACF;EACF;EACA,KAAK,MAAMA,GAAG,IAAImB,eAAe,EAAE;IACjC,IAAIG,IAAI,GAAGxC,UAAU,CAACiC,GAAG,CAACf,GAAG,CAAC;IAC9B,IAAI,CAACsB,IAAI,EAAE;MACTxC,UAAU,CAAC2C,GAAG,CAACzB,GAAG,EAAGsB,IAAI,GAAG,IAAIzC,GAAG,CAAC,CAAE,CAAC;MAEvC,IAAI,CAACK,UAAU,EAAE;QACfD,UAAU,CAACgB,GAAG,CAACD,GAAG,CAAC;MACrB,CAAC,MAAM;QACLhB,OAAO,CAACiB,GAAG,CAACD,GAAG,CAAC;MAClB;IACF;IAEAsB,IAAI,CAACrB,GAAG,CAACiB,WAAW,CAAC;EACvB;EAEAvC,UAAU,CAAC8C,GAAG,CAACP,WAAW,EAAEC,eAAe,CAAC;AAC9C;AAEA,SAASK,oBAAoBA,CAAClB,QAAQ,EAAE,MAAM,EAAEN,GAAG,EAAE,MAAM,EAAE;EAC3D,MAAMsB,IAAI,GAAGxC,UAAU,CAACiC,GAAG,CAACf,GAAG,CAAC,IAAInB,GAAG,CAAC,MAAM,CAAC;EAC/CyC,IAAI,CAACI,MAAM,CAACpB,QAAQ,CAAC;EAErB,IAAIgB,IAAI,CAACK,IAAI,KAAK,CAAC,EAAE;IACnB7C,UAAU,CAAC4C,MAAM,CAAC1B,GAAG,CAAC;IAEtB,IAAI,CAACd,UAAU,EAAE;MACfD,UAAU,CAACyC,MAAM,CAAC1B,GAAG,CAAC;IACxB,CAAC,MAAM;MACLhB,OAAO,CAAC4C,OAAO,CAAC5B,GAAG,CAAC;IACtB;EACF;AACF;AAEA,SAASF,WAAWA,CAACQ,QAAQ,EAAE,MAAM,EAAE;EACrC,MAAMgB,IAAI,GAAG3C,UAAU,CAACoC,GAAG,CAACT,QAAQ,CAAC;EACrC,IAAI,CAACgB,IAAI,EAAE;EAEX,KAAK,MAAMtB,GAAG,IAAIsB,IAAI,EAAE;IACtBE,oBAAoB,CAAClB,QAAQ,EAAEN,GAAG,CAAC;EACrC;EACArB,UAAU,CAAC+C,MAAM,CAACpB,QAAQ,CAAC;AAC7B;AAEA,SAASjB,eAAeA,CAAA,CAAE,EAAE,GAAG,CAAC;EAC9B,MAAMwC,OAAO,GAAGtD,aAAa,CAACuD,MAAM,CAACC,IAAI,CAACC,GAAG,CAAC;EAE9C,IAAI;IACF,OAAOC,OAAO,CAACC,GAAG,CAACC,gBAAgB,GAC/BN,OAAO,CAAC,UAAU,CAAC,GACnBO,QAAQ,CAACH,OAAO,CAACI,QAAQ,CAACC,IAAI,CAAC,IAAI,CAAC,GACpCT,OAAO,CAAC,UAAU,CAAC,GACnBA,OAAO,CAAC,4BAA4B,CAAC;EAC3C,CAAC,CAAC,OAAOU,GAAG,EAAE;IACZpC,OAAO,CAACqC,KAAK,CACX,yEAAyE,GACvE,6DACJ,CAAC;IACD,MAAMD,GAAG;EACX;AACF",
Copy link
Member Author

@nicolo-ribaudo nicolo-ribaudo Aug 27, 2024

Choose a reason for hiding this comment

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

The difference here is that a KAAK is replaced by EAAE,GAAG. KAAK means [+5, +0, +0, +0] and EAAE,GAAG means [+2,0,0,+2], [+3,0,0,+3], so the result is exactly the same.

The same is happening in the other source map diff.

@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Aug 28, 2024

@JLHwung @liuxingbaoyu This is ready for review! The main logic is in the new findToken&co functions in babel-generator/src/printer.ts, and the rest of the files in the generator/ folder have some extra logic for those cases where the "automatic" logic is not smart enough.

There are still 3 PRs it depends on at https://github.com/babel/babel/milestone/61, for now I have just cherry-picked them.

The tests are only failing on windows, I'll investigate why.

@nicolo-ribaudo nicolo-ribaudo marked this pull request as ready for review August 28, 2024 15:23
@nicolo-ribaudo

This comment was marked as resolved.

@nicolo-ribaudo nicolo-ribaudo force-pushed the wip-preserve-format branch 2 times, most recently from c85717b to 236fe25 Compare August 29, 2024 12:20
@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Aug 29, 2024

All the dependency PRs have been merged :)

When reviewing, I recommend checking packages/babel-generator/src/printer.ts first and only after the rest.

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.

It would be nice if we could somehow detect whether createParenthesizedExpressions was enabled correctly.

packages/babel-generator/src/printer.ts Outdated Show resolved Hide resolved
packages/babel-generator/src/printer.ts Outdated Show resolved Hide resolved
packages/babel-generator/src/generators/expressions.ts Outdated Show resolved Hide resolved
packages/babel-generator/src/printer.ts Outdated Show resolved Hide resolved
packages/babel-generator/src/printer.ts Outdated Show resolved Hide resolved
packages/babel-generator/src/printer.ts Outdated Show resolved Hide resolved
@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Aug 30, 2024

To try this PR in a different repo:

  • Go in the PR "Checks" for the latest commit that passed, in the "E2E tests" workflow (not the breaking one)
  • Download the "verdaccio-workspace" artifact
  • It contains tgz archives for all the packages. You only need the one for @babel/generator
  • Tell Yarn (or any other package manager) to use that archive, with "resolutions": { "@babel/generator": "./generator-7.25.7.tgz" }. You might also need to add "resolutions": { "@babel/parser": "^7.25.0" }
  • Enable the preserveFormat: true options of @babel/generator, and pass the original source code as the third parameter. Enable the tokens: true, createParenthesizedExpression: true options of @babel/parser.

@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Aug 30, 2024

It would be nice if we could somehow detect whether createParenthesizedExpressions was enabled correctly.

Maybe if we see extra.parenthesized somewhere we can error? I don't know how it interacts with nodes generated by @babel/template.

@liuxingbaoyu
Copy link
Member

liuxingbaoyu commented Aug 30, 2024

I added a rudimentary test in packages\babel-generator\test\preserve-format.js and it seems that currently there is some broken output for generated code without position information.
I am a little worried that there is something that is not caught by tests, maybe we can preserve a space for code without position information.

import { parse } from "@babel/parser";
import { cloneNode } from "@babel/types";
import path from "path";
import fixtures from "@babel/helper-fixtures";
import * as babel from "@babel/core";
import pluginTransformTypeScript from "@babel/plugin-transform-typescript";
import { commonJS } from "$repo-utils";

import _generate from "../lib/index.js";
const generate = _generate.default || _generate;

const { __dirname } = commonJS(import.meta.url);

const suites = (fixtures.default || fixtures)(path.join(__dirname, "fixtures"));

describe("preserveFormat", () => {
  describe("generation", () => {
    suites.forEach(testSuite => {
      describe(`${testSuite.title}`, () => {
        testSuite.tests.forEach(function (task) {
          if (task.options.throws) return;

          const testFn = task.disabled ? it.skip : it;

          testFn(task.title, () => {
            const input = task.actual.code.replace(/\r\n?/g, "\n");
            const parserOpts = {
              filename: task.actual.loc,
              plugins: task.options.plugins || [],
              strictMode: task.options.strictMode === false ? false : true,
              sourceType: "module",
              ...task.options.parserOpts,
              createParenthesizedExpressions: true,
              tokens: true,
            };
            const ast = parse(input, parserOpts);
            const options = {
              sourceFileName: path.relative(__dirname, task.actual.loc),
              ...task.options,
              retainLines: true,
              preserveFormat: true,
              comments: true,
              jsescOption: null,
              minified: false,
              compact: false,
            };

            const ok = generate(ast, options, input).code === input;
            const shouldFail = FAILURES.some(f =>
              task.actual.loc.replace(/\\/g, "/").endsWith(f),
            );

            if (!ok && shouldFail) {
              expect(1).toBe(1);
              return;
            }
            if (ok) {
              expect(shouldFail).toBe(false);
            }

            expect(generate(ast, options, input).code).toBe(input);
          });

          testFn(`${task.title} without loc`, () => {
            const input = task.actual.code.replace(/\r\n?/g, "\n");

            const parserOpts = {
              filename: task.actual.loc,
              plugins: task.options.plugins || [],
              strictMode: task.options.strictMode === false ? false : true,
              sourceType: "module",
              ...task.options.parserOpts,
              createParenthesizedExpressions: true,
              tokens: true,
            };
            const ast = parse(input, parserOpts);
            const options = {
              sourceFileName: path.relative(__dirname, task.actual.loc),
              ...task.options,
              retainLines: true,
              preserveFormat: true,
              comments: true,
              jsescOption: null,
              minified: false,
              compact: false,
            };

            ast.program = cloneNode(ast.program, true, true);

            const code = generate(ast, options, input).code;
            try {
              parse(code, parserOpts);
            } catch (error) {
              expect(code).toBe(input);
            }
          });
        });
      });
    });
  });

  describe("code transforms", () => {
    it("type stripping with arrow functions", () => {
      const input = `
        const a = (x: string):
                        number => 2 as const;
      `;
      const expected = `
        const a = (x        )=>
                                  2         ;
      `;

      const out = babel.transformSync(input, {
        configFile: false,
        plugins: [pluginTransformTypeScript],
        parserOpts: {
          createParenthesizedExpressions: true,
          tokens: true,
        },
        generatorOpts: {
          retainLines: true,
          preserveFormat: true,
        },
      });

      expect(out.code.trimEnd()).toBe(expected.trimEnd());
    });

    it("identifier renaming", () => {
      const input = `
        const foo = 3;
        const bar = 3;

        foo( x, y, z );
        bar( x, y, z );
      `;
      const expected = `
        const x   = 3;
        const longer=3;

        x  ( x, y, z );
        longer(x,y,z );
      `;

      const out = babel.transformSync(input, {
        configFile: false,
        plugins: [
          () => ({
            visitor: {
              Program(path) {
                path.scope.rename("foo", "x");
                path.scope.rename("bar", "longer");
              },
            },
          }),
        ],
        parserOpts: {
          createParenthesizedExpressions: true,
          tokens: true,
        },
        generatorOpts: {
          retainLines: true,
          preserveFormat: true,
        },
      });

      expect(out.code.trimEnd()).toBe(expected.trimEnd());
    });
  });
});

const FAILURES = [
  // These tests are either explicitly about re-formatting the decorators position,
  // or about an old decorators version
  "comments/decorators-after-export-to-before/input.js",
  "comments/decorators-before-export-to-after/input.js",
  "decorators/decorator-call-expression/input.js",
  "decorators/decorator-parenthesized-expression/input.js",
  "decorators/decorator-parenthesized-expression-createParenthesizedExpression/input.js",
  "decoratorsBeforeExport/false-to-true/input.js",
  "decoratorsBeforeExport/true-to-false/input.js",

  // The 'preserveFormat' option does not fully support Flow
  "flow/array-types/input.js",
  "flow/arrow-functions/input.js",
  "flow/call-properties/input.js",
  "flow/declare-module/input.js",
  "flow/declare-exports/input.js",
  "flow/declare-statements/input.js",
  "flow/indexed-access-types/input.js",
  "flow/interfaces-module-and-script/input.js",
  "flow/iterator-inside-declare/input.js",
  "flow/iterator-inside-interface/input.js",
  "flow/iterator-inside-types/input.js",
  "flow/object-literal-types/input.js",
  "flow/opaque-type-alias/input.js",
  "flow/parantheses/input.js",
  "flow/this-param/input.js",
  "flow/tuples/input.js",
  "flow/type-alias/input.js",
  "flow/type-annotations/input.js",
  "flow/type-union-intersection/input.js",
  "flow/typecasts/input.js",
  "flowUsesCommas/ObjectExpression/input.js",

  // These tests are explicitly about changing the output
  "importAttributesKeyword/assertions-assert-to-with/input.js",
  "importAttributesKeyword/assertions-assert-to-with-legacy/input.js",
  "importAttributesKeyword/assertions-with-to-assert/input.js",
  "importAttributesKeyword/assertions-with-to-default/input.js",
  "importAttributesKeyword/assertions-with-to-with-legacy/input.js",
  "importAttributesKeyword/attributes-assert-to-default/input.js",
  "importAttributesKeyword/attributes-assert-to-with/input.js",
  "importAttributesKeyword/attributes-assert-to-with-legacy/input.js",
  "importAttributesKeyword/attributes-with-to-assert/input.js",
  "importAttributesKeyword/attributes-with-to-default/input.js",
  "importAttributesKeyword/attributes-with-to-with-legacy/input.js",
  "importAttributesKeyword/legacy-module-attributes-to-assert/input.js",
  "importAttributesKeyword/legacy-module-attributes-to-with/input.js",

  // These tests are about old proposals
  "types/Decorator/input.js",

  // We explicitly always print `module X {}` as `namespace X {}`
  "typescript/module-namespace-head/input.js",
  "typescript/module-namespace-head-declare/input.js",
  "typescript/module-namespace-head-export/input.js",
  "typescript/export-declare/input.js",
];

@nicolo-ribaudo
Copy link
Member Author

@liuxingbaoyu Only one of those failures is an actual bug, fixed by #16788. The other are either:

  • for Flow, which preserveFormat does not support
  • for cases in which the generator "transpiles" code (for example, when it's configured to parse import assertions and emit import attributes) and thus it's missing the correct plugin to re-parse

You can see the actual error by replacing the assertion with expect([code, error]).toBe([input, null]);.

The mode without spaces is already used and well-tested, because it's the same logic that compact: true uses.

@liuxingbaoyu
Copy link
Member

Non-blocking: It seems that the benchmark (real-case) is 10% slower, I investigated and unfortunately did not find the reason.(Even if I replace all this.format.preserveFormat to false)

@nicolo-ribaudo
Copy link
Member Author

Oh, I'll take a look

@liuxingbaoyu
Copy link
Member

liuxingbaoyu commented Sep 2, 2024

This fixes the performance regression.
It seems the problem comes from closures, which is weird.

Thanks a lot for your commits, I used git bisect to finally find it.

diff --git a/packages/babel-generator/src/node/parentheses.ts b/packages/babel-generator/src/node/parentheses.ts
index 63db36c3ab..c9419bfe39 100644
--- a/packages/babel-generator/src/node/parentheses.ts
+++ b/packages/babel-generator/src/node/parentheses.ts
@@ -463,7 +463,11 @@ export function Identifier(
     }
   }
 
-  if (getRawIdentifier(node) !== node.name) {
+  const name =
+    typeof getRawIdentifier === "function"
+      ? getRawIdentifier(node)
+      : getRawIdentifier.name;
+  if (name !== node.name) {
     return false;
   }
 
diff --git a/packages/babel-generator/src/printer.ts b/packages/babel-generator/src/printer.ts
index 75f2e7e663..ce9c8acd61 100644
--- a/packages/babel-generator/src/printer.ts
+++ b/packages/babel-generator/src/printer.ts
@@ -879,9 +879,7 @@ class Printer {
         parent,
         this.tokenContext,
         this.inForStatementInit,
-        format.preserveFormat
-          ? node => this._getRawIdentifier(node)
-          : node => node.name,
+        format.preserveFormat ? this._getRawIdentifier.bind(this) : node,
       );
 
     if (

@nicolo-ribaudo
Copy link
Member Author

@liuxingbaoyu Perf regression fixed. I also added #16708 (comment) to the test suite.

@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Sep 3, 2024

@JLHwung @liuxingbaoyu I'm thinking of renaming this back to experimental_preserveFormat, until when the mode without retainLines: true (which can go in a separate release) is ready.

@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Sep 11, 2024

@JLHwung Done! While trying to figure out how to avoid the O(n^2) case, I changed the "find tokens corresponding to each node" logic to happen eagerly rather than lazily (since in practice we would do it for every node anyway), and that gave a 2x speedup when trying to print @babel/standalone's unminified bundle under this new mode (1s rather than 2s). I also extracted all of this logic to its own file, to make it easier to understand which methods are for tokens mappings and which are for normal printing.

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.

Incredibly amazing, thanks!
We can use if (this._tokenMap) instead of if (!this.format.preserveFormat), which is smaller and faster. :)

@nicolo-ribaudo
Copy link
Member Author

@liuxingbaoyu Done. I left .format.preserveFormat in a few places where we don't actually use the tokens, since it would have otherwise been confusing.

Thanks both for the reviews!

@nicolo-ribaudo nicolo-ribaudo added the PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release label Sep 11, 2024
@nicolo-ribaudo nicolo-ribaudo changed the title Add a format-preserving mode to @babel/generator Add experimental format-preserving mode to @babel/generator Sep 12, 2024
@nicolo-ribaudo nicolo-ribaudo merged commit 34c8793 into babel:main Oct 23, 2024
52 checks passed
@nicolo-ribaudo nicolo-ribaudo deleted the wip-preserve-format branch October 23, 2024 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: generator PR: New Feature 🚀 A type of pull request used for our changelog categories PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants