bufrw icon indicating copy to clipboard operation
bufrw copied to clipboard

Server-side bundling fails due to octals in ansi-color

Open shannonmoeller opened this issue 6 years ago • 15 comments

When trying to use bufrw (via thriftrw) in an environment where server side code is bundled with Babel and Webpack, the build process throws an error related to using Octal literals in strict mode:

error: ./node_modules/ansi-color/lib/ansi-color.js
Module build failed (from ./node_modules/fusion-cli/build/loaders/babel-loader.js):
SyntaxError: /var/tmp/project/node_modules/ansi-color/lib/ansi-color.js: Octal literal in strict mode (35:18)

  33 |   var ansi_str = "";
  34 |   for(var i=0, attr; attr = color_attrs[i]; i++) {
> 35 |     ansi_str += "\033[" + ANSI_CODES[attr] + "m";
     |                   ^
  36 |   }
  37 |   ansi_str += str + "\033[" + ANSI_CODES["off"] + "m";
  38 |   return ansi_str;
    at Parser.raise (/var/tmp/project/node_modules/@babel/parser/lib/index.js:6322:17)
    at Parser.readEscapedChar (/var/tmp/project/node_modules/@babel/parser/lib/index.js:7418:20)
    at Parser.readString (/var/tmp/project/node_modules/@babel/parser/lib/index.js:7265:21)
    at Parser.getTokenFromCode (/var/tmp/project/node_modules/@babel/parser/lib/index.js:6950:14)
    at Parser.nextToken (/var/tmp/project/node_modules/@babel/parser/lib/index.js:6520:12)
    at Parser.next (/var/tmp/project/node_modules/@babel/parser/lib/index.js:6460:10)
    at Parser.parseMaybeAssign (/var/tmp/project/node_modules/@babel/parser/lib/index.js:8210:12)
    at Parser.parseExpression (/var/tmp/project/node_modules/@babel/parser/lib/index.js:8120:23)
    at Parser.parseStatementContent (/var/tmp/project/node_modules/@babel/parser/lib/index.js:9892:23)
    at Parser.parseStatement (/var/tmp/project/node_modules/@babel/parser/lib/index.js:9763:17)
    at Parser.parseBlockOrModuleBlockBody (/var/tmp/project/node_modules/@babel/parser/lib/index.js:10340:25)
    at Parser.parseBlockBody (/var/tmp/project/node_modules/@babel/parser/lib/index.js:10327:10)
    at Parser.parseBlock (/var/tmp/project/node_modules/@babel/parser/lib/index.js:10311:10)
    at Parser.parseStatementContent (/var/tmp/project/node_modules/@babel/parser/lib/index.js:9839:21)
    at Parser.parseStatement (/var/tmp/project/node_modules/@babel/parser/lib/index.js:9763:17)
    at node.body.withTopicForbiddingContext (/var/tmp/project/node_modules/@babel/parser/lib/index.js:10374:60)
 @ ./node_modules/bufrw/error_highlighter.js 22:12-33
 @ ./node_modules/bufrw/interface.js
 @ ./node_modules/bufrw/index.js
 @ ./node_modules/thriftrw/index.js
 @ ./src/server/lib/thrift-http.js
 @ ./src/server/app.js

Are you open to a PR replacing the use of ansi-color with ansi-colors or chalk?

shannonmoeller avatar May 01 '19 16:05 shannonmoeller

Please reopen if this continues to be a problem. My impression is that the underlying problem with these tools has been addressed.

kriskowal avatar Aug 25 '19 23:08 kriskowal

@kriskowal this is still an issue with version 1.3.0.

For some reason the changes from #64 were reverted. And there is also the [email protected] dependency that still uses ansi-color.

johnmaia avatar Nov 13 '19 12:11 johnmaia

Thanks @kriskowal! This issue appeared to us because we have an application that has [email protected] as a dependency and [email protected] as a dev-dependency and when we attempt to run jest --coverage we get the following error:

Test suite failed to run

    SyntaxError: /Users/johnmaia/Projects/foobar/node_modules/ansi-color/lib/ansi-color.js: Octal literal in strict mode (35:18)

      33 |   var ansi_str = "";
      34 |   for(var i=0, attr; attr = color_attrs[i]; i++) {
    > 35 |     ansi_str += "\033[" + ANSI_CODES[attr] + "m";
         |                   ^
      36 |   }
      37 |   ansi_str += str + "\033[" + ANSI_CODES["off"] + "m";
      38 |   return ansi_str;

      at Parser.raise (node_modules/@babel/parser/lib/index.js:6420:17)
      at Parser.readEscapedChar (node_modules/@babel/parser/lib/index.js:7542:20)
      at Parser.readString (node_modules/@babel/parser/lib/index.js:7389:21)
      at Parser.getTokenFromCode (node_modules/@babel/parser/lib/index.js:7057:14)
      at Parser.nextToken (node_modules/@babel/parser/lib/index.js:6630:12)
      at Parser.next (node_modules/@babel/parser/lib/index.js:6559:10)
      at Parser.parseMaybeAssign (node_modules/@babel/parser/lib/index.js:8361:12)
      at Parser.parseExpression (node_modules/@babel/parser/lib/index.js:8275:23)
      at Parser.parseStatementContent (node_modules/@babel/parser/lib/index.js:10138:23)
      at Parser.parseStatement (node_modules/@babel/parser/lib/index.js:10009:17)

If we use an older version of jest like version 21.1.0 this is no longer an issue. We've tried to find a babel plugin that would handle this issue, given that we don't wish to downgrade the version of jest on all of our projects, but we had no success.

johnmaia avatar Nov 14 '19 11:11 johnmaia

The bottom line is that this is valid JavaScript and the bug is presumably in Babel. The line beneath the bottom line is that this could be addressed by using hex instead of octal in ansi-color. Perhaps attach related issues from those projects. I believe we would prefer to resolve this issue with changes to these dependencies.

Issue #64 was closed by the author who found a work-around. It was not landed nor reverted. If we had reviewed it, would would have balked at absorbing an adapter for ansi-colors. We would like to keep this library’s scope tight and work with our ecosystem to resolve issues closer to their source.

Thanks for tracking the issue. We are not planning changes, but will review suggestions. We are open to uprevving our dependencies as long as we preserve support for Node.js 0.10.

kriskowal avatar Nov 27 '19 19:11 kriskowal

I recently choose to try server-side bundling for some projects.

I would recommend parcel

parcel build index.js --bundle-node-modules --target node --no-minify --log-level 4

This does a great job of taking a simple program that you would run with node index.js and creating a single bundle in dist/index.js that can be run without node_modules or npm install with node dist/index.js

Raynos avatar Nov 28 '19 06:11 Raynos

I just tried parcel with bufrw and it generates a working bundle ; i'd second that this is an issue with babel and webpack ;

Raynos avatar Nov 28 '19 06:11 Raynos

The bottom line is that this is valid JavaScript

Yes, but non-normative and discouraged JavaScript.

would have balked at absorbing an adapter for ansi-colors

As you should have. It's why I also opened loopj/commonjs-ansi-color#14.

shannonmoeller avatar Nov 30 '19 03:11 shannonmoeller

@johnmaia Thanks for your workaround!

when I run jest --coverage, I found the same issue on my project. Besides using an old version of jest, do you found any other solutions?

Ricardo-Li avatar Nov 01 '20 09:11 Ricardo-Li

Hi, I have this issue while trying to use OpenTelemetry code in my application. I just want my perfectly functioning code to work and not be hobbled by dependencies I didn't install.

@Ricardo-Li we ended up dropping the package that depended on bufrw so other than downgrading jest I don't have a better solution.

johnmaia avatar Dec 02 '20 11:12 johnmaia

Have you tried opening a bug report on the jest project ?

Raynos avatar Dec 02 '20 11:12 Raynos

It's not a jest issue. I don't understand the resistance to fixing this. It's a small change and I already did the hard part if an adapter is okay (#64). Otherwise using a different dependency would also fix it.

shannonmoeller avatar Dec 02 '20 16:12 shannonmoeller

If anyone is reaching this bug due to OpenTelemetry or jest -- specifically, due the OpenTelemetry SDK transitively depending on this project via jaeger-client -- the fix is to make sure that the node_modules directory is properly excluded from jest coverage computation (via the coveragePathIgnorePatterns or collectCoverageFrom options).

artdent avatar Jan 11 '23 14:01 artdent

If anyone is reaching this bug due to OpenTelemetry or jest -- specifically, due the OpenTelemetry SDK transitively depending on this project via jaeger-client -- the fix is to make sure that the node_modules directory is properly excluded from jest coverage computation (via the coveragePathIgnorePatterns or collectCoverageFrom options).

Also check does you collect coverage from *.js files. I had the same issue, ignoring *.js files helped me. I use typescript instead.

Insem avatar Apr 07 '23 10:04 Insem