-
Notifications
You must be signed in to change notification settings - Fork 231
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
Build for Deno #452
Build for Deno #452
Conversation
Anyone that can help me debug this fails? I tried to test them on Node 6.17.1 and they all passed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes done inside lib
must be done inside build/
instead. There is a bunch of scripts there that takes code from Node.js core and rewrite it so it's agnostics. We could potentially use some of the same machinery to generate a deno build.
(Unfortunately I do not have time to debug the build right now, I'm going for some extended leave and I'll be back full time next year). |
@mcollina can you point me at the code that generates readable-stream from Node core? I'm not actually familiar with it :]? |
Someone knows what version of Node the last build of readable stream is using? The README points out to it being Node 10 but I can't really tell (needed for running the build script) |
Looking at the issue I think it's v10.18.1 |
Thanks @benjamingr |
This seems pretty amazing! Are there any tests for this? |
@mcollina Actually...that's the main reason why there's been so little activity in this since it was created. The build has been working for a while, but making the tests work has proven to be a major headache due to the differences between the Deno test suite and something like tap More progress to follow though, I'm on vacations so I am putting some effort into landing this |
You can also write a few high level tests. We run separate tests for the browser for this reason, see https://github.com/nodejs/readable-stream/tree/master/test/browser for more details. |
FYI: check out #454. |
I doubt that Deno won't get much traction for node streams, they are mostly just focused on web standarder, meaning: whatwg streams. Node also recently got them. I always try to avoid core builtin's when i develop anything for cross platform that runs in Deno, Browser and Node. |
Discussion in #451
This needs some polishing, namely: