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

rfc: no comment purge #1098

Closed
cdaringe opened this issue Apr 1, 2021 · 3 comments
Closed

rfc: no comment purge #1098

cdaringe opened this issue Apr 1, 2021 · 3 comments

Comments

@cdaringe
Copy link

cdaringe commented Apr 1, 2021

Problem

JavaScript/TypeScript users use comments for a variety of reasons, including the PURE tree shaking case.

A key usage I use them for is istanbul ... comments, which is critical for testing & coverage. Stripping these comments makes using esbuild for testing + coverage infeasible in large codebases, where such comment flags are actually critical.

Discussion

This project has a strong philosophy of keeping features low. Love it.

Is not deleting content in or out of scope for feature support? I would like to have a flag for never deleting comments. I'd advocate that we should consider this, because at the core, the current compiler is lossy. This feature isn't about supporting some new fancy language feature--it's just keeping existing content in place, especially content that is already understood and handled by the parser and printer.

@evanw
Copy link
Owner

evanw commented Apr 1, 2021

Is not deleting content in or out of scope for feature support?

Comments are by definition not part of the content. From the JavaScript standard itself:

Simple white space and single-line comments are discarded and do not appear in the stream of input elements for the syntactic grammar. A MultiLineComment (that is, a comment of the form /*...*/ regardless of whether it spans more than one line) is likewise simply discarded if it contains no line terminator; but if a MultiLineComment contains one or more line terminators, then it is replaced by a single line terminator, which becomes part of the stream of input elements for the syntactic grammar.

What you are actually asking for is for esbuild to assign specific semantics to certain comments for a specific tool that you want to use, to attach those annotations to the AST according to those semantics, and then for esbuild to preserve those semantics as it compiles and transforms your code. Doing that would add a lot of work and ongoing maintenance burden and I have decided that it's out of scope. You can read more about this here: #578 (comment).

@revelt
Copy link

revelt commented Apr 3, 2021

In theory, if we had a plugin like rollup-plugin-cleanup (which respects istanbul, when called like cleanup({ comments: "istanbul", extensions: ["js", "ts"] }),), then another for console.* removal (alternative of @rollup/plugin-strip), then another plugin to mildly minify (no touching of comment lines or comments), then disabled esbuild's built-in minification, that would be a possible solution? What do you think? A plugins approach...

I'm also struggling with console.log and comments stripping challenge.

@evanw
Copy link
Owner

evanw commented Apr 25, 2021

I'm going to close this for the reason described above: I consider this to be out of scope.

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

No branches or pull requests

3 participants