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

Nested comments aren't emitted correctly #5787

Closed
jam13 opened this issue Feb 5, 2021 · 6 comments
Closed

Nested comments aren't emitted correctly #5787

jam13 opened this issue Feb 5, 2021 · 6 comments

Comments

@jam13
Copy link

jam13 commented Feb 5, 2021

🐛 bug report

I've been testing nightly builds in order to make use of some updates that aren't in 2.0.0-beta.1, and been having build problems. They occur when trying to bundle the typescript monaco worker (all other workers build fine) as outlined here:

https://github.com/microsoft/monaco-editor/blob/master/docs/integrate-esm.md#using-parcel

When building, it fails at the optimise step due to the js from the previous step containing a syntax error.

This works fine on beta.1, and I've narrowed it down to 2.0.0-nightly.522 which has the following two commits on top of 520:

If I disable minification then the build succeeds, but the js contains the syntax error.

🎛 Configuration (.babelrc, package.json, cli command)

I can reproduce with the default parcel config and no babelrc.

parcel build ./node_modules/monaco-editor/esm/vs/language/typescript/ts.worker.js --no-source-maps

🤔 Expected Behaviour

Optimised ts.worker.js bundle should be written to dist

😯 Current Behaviour

$ yarn monaco:worker:ts
yarn run v1.22.10
$ parcel build ./node_modules/monaco-editor/esm/vs/language/typescript/ts.worker.js --no-source-maps --dist-dir dist/monaco
🚨 Build failed.
@parcel/optimizer-terser: Unexpected token: operator (*)
  19028 |           /** @returns {number}*/
> 19029 |           /**/*/
>       |              ^ Unexpected token: operator (*)
  19030 |           /*var x = function(name) { return name.length; }*/
  19031 | } else // Try to recognize this pattern when node is initializer of variable declaration and JSDoc comments are on containing variable statement.
It's likely that Terser doesn't support this syntax yet.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

💁 Possible Solution

The problem might be due to missing whitespace and/or newlines around comments in the bundled js, as this seems to make up the majority of the diff between the two unminified versions of 520 and 522. The syntax error also looks like it occurs within a mangled comment.

🔦 Context

Can't use nightly > 520

🌍 Your Environment

Software Version(s)
Parcel >=2.0.0-nightly.522
Node 14.15.4
Yarn 1.22.10
Operating System MacOS
@mischnic
Copy link
Member

mischnic commented Feb 5, 2021

Minimal reproduction:

if (1) {
    a();
}
// /* */
else {
    b();
}

becomes

(function () {
  if (1) {
    a();
      /*/* */*/
} else // /* */
  {
    b();
  }
})();

@mischnic
Copy link
Member

mischnic commented Feb 5, 2021

A workaround would be to remove that comment in monaco-editor/esm/vs/language/typescript/lib/typescriptServices.js:
search for Try to recognize this pattern when node and remove the block comment inside the line comment.

@jam13
Copy link
Author

jam13 commented Feb 5, 2021

A workaround would be to remove that comment in monaco-editor/esm/vs/language/typescript/lib/typescriptServices.js:
search for Try to recognize this pattern when node and remove the block comment inside the line comment.

Thanks for the minimal test case. I'm ok to stick on the previous nightly for now, but will try this to confirm it works.

@ittaibaratz
Copy link
Contributor

We had the same issue caused by highlight.js@10.3.1 . Bumping to 10.6.0 solved the problem.

@mischnic mischnic changed the title Invalid js produced by bundler Nested comments aren't emitted correctly Feb 19, 2021
@mischnic
Copy link
Member

But the comment in the test is still duplicated because of Babel's AST format
(#5825)

Babel has recently changed the logic to check for comment identity (so a WeakSet to ensure each comment object is only printed once) instead of deduplicating via the source location. That should work in our case as well

@mischnic
Copy link
Member

This should be fixed in the recent versions using swc

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

No branches or pull requests

3 participants