🏗🚮 Clean up closure compiler code paths no longer used by amp dist

[rsimha]

The coast is clear to delete all the closure compilation code paths used during minification. The guts of the actual compilation still remain because they're used during type checking. They can be cleaned up after type checking moves to TS.

Related to #35264

[samouri]

Lets wait until next week when the esbuild build hits stable. In case we need to revert, deleting this code prematurely could get ugly.

That said, I'm very excited about all the deletion we can do :)

[rileyajones]

Awesome cleanup! I second @samouri 's suggestion to wait until esbuild is in stable though

[jridgewell]

Blocking until esbuild+terser is stable.

[kristoferbaxter]

Looks good to me too, but agree with @jridgewell about holding off until stable.

Verification question too: do we have checks for the quality of sourcemaps with this change applied? I believe we do, but just want to verify as there are MagicString removals.

[samouri]

New stable is now live, built with esbuild.
Good to merge ⭐

Stable

[rsimha]

Verification question too: do we have checks for the quality of sourcemaps with this change applied? I believe we do, but just want to verify as there are MagicString removals.

We have amp check-sourcemaps which inspects various fields in the sourcemap file for module and nomodule output. Anything else you think we need?

[kristoferbaxter]

The on-call folks noticed that sourcemaps didn't match after the esbuild version was shipped to production traffic. Perhaps there is some additional coverage needed? Might be worth syncing with @jridgewell or @rcebulko.

[samouri]

@kristoferbaxter / @rsimha: the current check-sourcemaps verification is quite coarse. It works well for ensuring our sourcemaps are at least somewhat correct. Maybe we can modify the check to also look for specific identifiers which would give us significantly more confidence.

The current issue we've noticed is that StackDriver error reports are displaying traces with _AMP_PRIVATE_, which is a suffix we give to private identifiers during a babel pass. That identifier should be mapped back to the original source instead of the post-babel name. We are still investigating why this is happening, and Justin has a PR that might fix it: #37238. The current theory is that terser isn't using the sourcemap we hand it from babel properly.