-
Notifications
You must be signed in to change notification settings - Fork 1
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
Buffer is not defined #42
Comments
@Tpt Since you worked on this recently, do you have any additional insights about this issue? |
Yes, the |
But |
@surilindur Can you check what happens if we add |
Indeed, that's a good point. I missed that sorry. |
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 If that's not the case yet, @surilindur I would suggest adding a polyfill for |
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. |
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). |
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) |
I have opened #43 that migrates to json-event-parser (good test for the library) |
Very nice, thanks @Tpt! (please remind me to review that PR once we release |
Temporary workaround for rubensworks/sparqljson-parse.js#42
@surilindur FYI, this PR was just merged: nodejs/readable-stream#489 |
I tried to look at what the changes in the PR do, but it seems like it only defines the So it does fix the need for polyfills, but only for |
I guess that should be sufficient?
Do you know which other libs still depend on |
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 |
That's great news, thanks for checking @surilindur! |
Temporary workaround for rubensworks/sparqljson-parse.js#42
When used in a browser without Node.js polyfills, the
jsonparse
library produces an error aboutBuffer
not being defined.The error comes from here, in the
Parser
function: https://github.com/creationix/jsonparse/blob/1.3.1/jsonparse.js#L59But 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:So whenever the
Parser
function is called, it attempts to access theBuffer
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?
The text was updated successfully, but these errors were encountered: