Buffer is not defined
Closed this issue · 16 comments
When used in a browser without Node.js polyfills, the jsonparse library produces an error about Buffer not being defined.
The error comes from here, in the Parser function: https://github.com/creationix/jsonparse/blob/1.3.1/jsonparse.js#L59
But there are also other locations where the global is used.
The library exports the Parser function at the end, which is then imported here and called in two locations, producing the error:
- https://github.com/rubensworks/sparqljson-parse.js/blob/master/lib/SparqlJsonParser.ts#L45
- https://github.com/rubensworks/sparqljson-parse.js/blob/master/lib/SparqlJsonParser.ts#L137
So whenever the Parser function is called, it attempts to access the Buffer global by the looks of it.
Maybe that global could somehow be provided by hand, when there is no Webpack or other means to do it? Or maybe the Buffer could be directly replaced with Uint8Array if it does not use any methods from the Node subclass?
@Tpt Since you worked on this recently, do you have any additional insights about this issue?
Yes, the buffer package should be installed for the webpack build to work. readable-stream requires it to target browsers but does not explicitly requires it.
But buffer already is a dependency of this project, right?
@surilindur Can you check what happens if we add buffer as a direct dependency to https://github.com/comunica/jQuery-Widget.js ?
But buffer already is a dependency of this project, right?
Indeed, that's a good point. I missed that sorry. jsonparse seems to assume that Buffer is global. It's maybe not the case in @surilindur's project. But I don't find any place where Buffer is declared as global in this package CI Webpack build. So I am not sure what is working here that is not working in @surilindur's project. rdfxml-streaming-parser.js seems to have the same behavior.
But I don't find any place where Buffer is declared as global in this package CI Webpack build.
Perhaps the build process itself runs fine, but problems may only arise when using the webpacked code.
@Tpt Since you were working on a fork of jsonparse in https://github.com/comunica/json-event-parser.js, do you think that's in a usable enough state already to use in here as well?
If that's not the case yet, @surilindur I would suggest adding a polyfill for buffer in the webpack config for now until we find a solution to the problem.
Okay! Thank you both for input. I will see where and how to add it.
And yes, the problem seems to be the use of the global, without it being defined anywhere for the browser, and it only throws an error when actually running the code that tries to use it.
@Tpt Since you were working on a fork of jsonparse in https://github.com/comunica/json-event-parser.js, do you think that's in a usable enough state already to use in here as well?
Yes, it should work and bring real streaming. I managed to get the JSON-LD parser working on top of it (but I needed to keep the JSON content inside of the JSON-LD parser to keep all its behaviors working).
but I needed to keep the JSON content inside of the JSON-LD parser to keep all its behaviors working
Yeah, that's what I suspected. We may have to make some changes in the JSON-LD parser to not require this persistence of all JSON content, as we'll run into memory issues for long/infinite JSON-LD documents.
(Probably should discuss this elsewhere)
Very nice, thanks @Tpt!
(please remind me to review that PR once we release json-event-parser :-) )
@surilindur FYI, this PR was just merged: nodejs/readable-stream#489
Could you check if this resolves all of our problems, and if our workaround can be undone?
I tried to look at what the changes in the PR do, but it seems like it only defines the Buffer variable internally in the readable-stream library for the files that depend on it, and does not define it globally. I also tested it by trying to squeeze the latest version everywhere possible but it did not make a difference. 😦
So it does fix the need for polyfills, but only for readable-stream itself, not anything else that requires Buffer.
but it seems like it only defines the Buffer variable internally in the readable-stream library for the files that depend on it, and does not define it globally.
I guess that should be sufficient?
I also tested it by trying to squeeze the latest version everywhere possible but it did not make a difference.
Do you know which other libs still depend on buffer?
Actually, I tested with the wrong configuration, my bad, I completely forgot #43 and the changes there. With those changes, the workaround can be removed, as long as the new json-event-parser library uses the ^4.2.0 version of readable-stream. This is awesome. Apologies again. 😓
That's great news, thanks for checking @surilindur!