browserify / static-module

convert module usage to inline expressions
MIT License
74 stars 23 forks source link

Apparent Timing Issue Between Streams? #14

Closed dahjelle closed 6 years ago

dahjelle commented 9 years ago

I've been trying to run browserify and brfs on pdfkit, but running into the issue at substack/brfs#23. Digging through things a bit, it seems that there is an odd timing issue with streams such that replacing

s.stream.on('end', next);

(from index.js) with

s.stream.on('end', function() {
  setTimeout(next, 0);
});

seems to fix the issue in this case, though it certainly isn't a very satisfying fix.

More background:

$ npm ls brfs browserify pdfkit
~/brfs
├── brfs@1.4.0
├── browserify@10.2.4
└── pdfkit@0.7.1

I'm testing by running

$ node_modules/.bin/browserify -t brfs node_modules/pdfkit/js/font.js > test.js

Without the fix, I get the error:

SyntaxError: Unterminated string constant (92:15) while parsing file: /Users/dahjelle/Downloads/brfs/node_modules/pdfkit/js/font.js
    at Parser.pp.raise (/Users/dahjelle/Downloads/brfs/node_modules/brfs/node_modules/static-module/node_modules/falafel/node_modules/acorn/dist/acorn.js:1745:13)
    at Parser.pp.readString (/Users/dahjelle/Downloads/brfs/node_modules/brfs/node_modules/static-module/node_modules/falafel/node_modules/acorn/dist/acorn.js:3656:31)
    at Parser.pp.getTokenFromCode (/Users/dahjelle/Downloads/brfs/node_modules/brfs/node_modules/static-module/node_modules/falafel/node_modules/acorn/dist/acorn.js:3446:19)
    at Parser.pp.readToken (/Users/dahjelle/Downloads/brfs/node_modules/brfs/node_modules/static-module/node_modules/falafel/node_modules/acorn/dist/acorn.js:3189:15)
    at Parser.pp.nextToken (/Users/dahjelle/Downloads/brfs/node_modules/brfs/node_modules/static-module/node_modules/falafel/node_modules/acorn/dist/acorn.js:3181:71)
    at Parser.pp.next (/Users/dahjelle/Downloads/brfs/node_modules/brfs/node_modules/static-module/node_modules/falafel/node_modules/acorn/dist/acorn.js:3130:8)
    at Parser.pp.parseReturnStatement (/Users/dahjelle/Downloads/brfs/node_modules/brfs/node_modules/static-module/node_modules/falafel/node_modules/acorn/dist/acorn.js:2553:8)
    at Parser.pp.parseStatement (/Users/dahjelle/Downloads/brfs/node_modules/brfs/node_modules/static-module/node_modules/falafel/node_modules/acorn/dist/acorn.js:2427:19)
    at Parser.pp.parseBlock (/Users/dahjelle/Downloads/brfs/node_modules/brfs/node_modules/static-module/node_modules/falafel/node_modules/acorn/dist/acorn.js:2692:21)
    at Parser.pp.parseFunctionBody (/Users/dahjelle/Downloads/brfs/node_modules/brfs/node_modules/static-module/node_modules/falafel/node_modules/acorn/dist/acorn.js:1413:22)
    at Parser.pp.parseFunction (/Users/dahjelle/Downloads/brfs/node_modules/brfs/node_modules/static-module/node_modules/falafel/node_modules/acorn/dist/acorn.js:2770:8)
    at Parser.pp.parseExprAtom (/Users/dahjelle/Downloads/brfs/node_modules/brfs/node_modules/static-module/node_modules/falafel/node_modules/acorn/dist/acorn.js:1129:19)

This seems to happen because one of the fs.readFileSync replacements gets cut off in the middle in the first pass (i.e. the pass as specified in pdfkit's package.json), and so the second pass (i.e. specified in the command line) doesn't have valid syntax to parse any longer.

It is as if the output stream continues before the s.stream is fully piped into it. No idea exactly why the setTimeout fixes it, or if it fixes all cases, but it sure seems to fix this one.

dahjelle commented 9 years ago

FWIW, this occurs in Node 0.10.36, 0.10.39, and 0.12.6, as well as in io.js 2.3.3 with the same symptoms and "workaround".

nmccready commented 9 years ago

also happens on 0.12.7 with exact same error and file font.js, seeing error in 4.1.0 as well

goto-bus-stop commented 6 years ago

I think what happens is this:

likely fix is to use output.write instead of output.push.