search

hildjj / node-cbor

Encode and decode CBOR documents, with both easy mode, streaming mode, and SAX-style evented mode.
MIT License
356 stars 73 forks source link

cbor-web: Unexpected token on rollup build, is `for(;;)` supposed to be here? #183

Closed lemoustachiste closed 1 year ago

lemoustachiste commented 1 year ago

Hi,

so I'm trying to consume the cbor-web package instead of borc. However when I'm bundling my project with rollup, I get the following error:

SyntaxError: Unexpected token (6:82763) in /project/node_modules/cbor-web/dist/cbor.js

When looking at what's at this index, it somewhat looks normal, but a few characters before this empty for loop is there, and I'm wondering if that's actually supposed to be here. (It is there in the github repo, if you search for for(;;)).

Such syntax in a REPL creates an infinite loop so it looks weird to me.

hildjj commented 1 year ago

Can you pretty-print the JS, and send me a relevant snippet of code around the for loop, please?

hildjj commented 1 year ago

(I should have said pretty please. :) )

hildjj commented 1 year ago

Nevermind. This is probably the while(true) loop in Decoder._parse(). It's fine. Decoder is a stream that may or may not ever end, by design.

lemoustachiste commented 1 year ago

Yeah it looks like code optimization. If you look at the MDN docs, for(;;) is valid syntax (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for), provided the code has a way to break the loop.

So it's not my actual problem. I'm going to investigate a bit more and let you know.

lemoustachiste commented 1 year ago

The problem comes from a dependency of the unmaintained rollup-plugin-node-globals package which I need.

I am forking the package to update the dep, so not a problem with node-cbor as far as I can tell now

hildjj commented 1 year ago

Let me know if you run into issues. I'm also getting close on a replacement library for node-cbor which has no runtime dependencies and is all ES6 -- it should be easier to drop right into your workflow.

lemoustachiste commented 1 year ago

Thanks, I'll keep an eye out

hildjj commented 1 year ago

Just released v0.0.1 of cbor2 yesterday: https://github.com/hildjj/cbor2/ You should just be able to npm install, import it into your rollup project, then let rollup bundle it and tree shake it into your project.

I'm interested in opinions either way.