Skip to content

rollup_bundle loses sourcemap context due to running tsc for downleveling #175

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

Closed
mhevery opened this issue Apr 4, 2018 · 6 comments
Closed
Labels
Milestone

Comments

@mhevery
Copy link
Contributor

mhevery commented Apr 4, 2018

Here is my understand of how rollup_bundle works:

  1. copy files into bundles.es6 directory
  2. Rollup the files in bundles.es6 directory into bundle.es6.js file
  3. Downlevel the file from ES6 -> ES5 into bundle.js using tsc
  4. Run uglify to generate bundle.min.js

The pipeline above works fine for the source code, but it brakes source map. There are 2 issues:

  1. Many of the actions do not declare *.js.map as output of the action. This can be easily fixed as shown here
  2. The source map pipeline is broken. The reason for this is that when tsc is used to downlevel the code from ES2015->ES5 tsc assumes that bundle.es6.js is the source file. This is incorrect assumption. Instead tsc should transform the source maps in bundel.es6.js to the next step. I am not even sure if tsc supports such mode. In any case the issue is that at this point the source map have been destroyed and so the subsequent source maps are of no value.
@alexeagle
Copy link
Collaborator

Right, typescript is a broken downleveler for generated inputs.
The ng_package rule solves this by re-building the dependencies with typescript to esm5.

@mhevery
Copy link
Contributor Author

mhevery commented Apr 4, 2018

After discussion with @IgorMinar we feel that the correct path should be:

  1. Using tsc generate bundle.esm5 directory with files
  2. Rollup bundle.esm5 into bundle.js
  3. Run uglify to convert bundle.js into bundle.js.min

@alexeagle alexeagle changed the title rollup_bundle rule does not correctly retain source maps. rollup_bundle loses sourcemap context due to running tsc for downleveling Apr 9, 2018
@alexeagle
Copy link
Collaborator

I fixed this for ng_rollup_bundle by using the hacky esm5.bzl we have in the Angular repo to get a downleveled ESM file straight from TypeScript. But this is a bunch of extra complexity.

One option is to make downleveled-ESM an official third output flavor from ts_library but @mprobst has rejected this idea in the past because of increased complexity in the TypeScript rules, which we don't need internally at Google (because closure compiler does the downleveling itself)

Another option is to push on microsoft/TypeScript#13944 so that TypeScript can be used to downlevel generated JS files.

Another option is to find some other toolchain that does all these steps and handles sourcemaps correctly.

I haven't heard this requested from anyone so I think this issue is low priority.

@connorjclark
Copy link
Contributor

babel accepts input source maps. Any reason tsc must be used for downleveling?

@mprobst
Copy link
Contributor

mprobst commented Sep 25, 2019 via email

@alexeagle
Copy link
Collaborator

Good timing @connorjclark I just checked in a test yesterday that the new rollup/terser rules compose sourcemaps correctly.

Should have closed this issue as part of #1181

Note, we did switch from tsc to babel as the "downlevel" step here, but note that the feature now lives entirely in user-space - we don't have a babel rule or any interop with it.

We are working to add the proper JS provider so that TypeScript outputs play correctly with rollup_bundle and other rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants