Skip to content
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

Browserifiability #88

Closed
rvagg opened this issue Mar 29, 2014 · 12 comments
Closed

Browserifiability #88

rvagg opened this issue Mar 29, 2014 · 12 comments

Comments

@rvagg
Copy link
Member

rvagg commented Mar 29, 2014

@substack @calvinmetcalf @feross @maxogden

I've merged all the PRs and synchronised the build scripts between the master and v0.10 branches--this was causing major pain in getting everything merged so I wouldn't be surprised if some things didn't go in properly.

Could you each check over both the master and v0.10 branches and see if they're doing what you wanted to be done for browserifiability for both 1.1 and 1.0 releases of readable-stream?

@rvagg
Copy link
Member Author

rvagg commented Mar 29, 2014

/cc @Raynos

@calvinmetcalf
Copy link
Contributor

it looks like assert is in writable in the v0.10 branch but isn't actually being used anywhere, it's not in master.

@rvagg
Copy link
Member Author

rvagg commented Mar 30, 2014

great pick-up @calvinmetcalf, I'll deal with that. Can you confirm that your changes made it in? that was kind of messy to merge on top of the other changes and back in to v0.10 too.

@rvagg
Copy link
Member Author

rvagg commented Mar 30, 2014

I've published a new version of readable-stream with these updates. 1.0.26-3 is the current latest version but there is also now a 1.1.12 that is on the 1.1 tag (i.e. you have to explicitly install a streams3/v0.11 version if you want it for now).

Let me know if there's anything else that needs to happen on this front.

@ghost
Copy link

ghost commented May 10, 2014

I've got readable-stream working in browsers including old ones but I'm having a lot of trouble porting core browserify to supply readable stream in the require('stream') case but the require('stream') inside readable-stream itself is causing a lot of issues with cyclic inheritance. Can we just get rid of that part? People should expect that instanceof should break anyways since it's basically incompatible with how npm works on a fundamental level.

@ghost
Copy link

ghost commented May 10, 2014

browserify 4.0 and up is using readable-stream now with just a small shim in stream-browserify to bootstrap the classic mode pipe implementation just like in node core.

@rvagg
Copy link
Member Author

rvagg commented Aug 1, 2014

anything we need to do here on the browserify compatibility front before a 1.0.30 release now that we have a small window for a release?

@AutoSponge
Copy link

I've noticed, while using jspm, that the browserified version of 1.13 fails to import. I traced it to this line:

https://github.com/iojs/readable-stream/blob/3b672fd7ae92acf5b4ffdbabf74b372a0a56b051/lib/_stream_readable.js#L54

which translates to:

var debug = require('@empty');

After changing the file to:

var debug = require('util');

Everything works as expected. I'm guessing that util is not in the deps list and jspm can't map it. Is this something to fix here or in browserify or jspm?

@calvinmetcalf
Copy link
Contributor

util was not supposed to be imported, its set to false in the package.json, likely an jspm issue would be my guess

@sonewman
Copy link
Contributor

@AutoSponge if you required util that would include more than the debug, also in the code where it calles debug(*something*) wouldn't that throw TypeError: object is not a function?

@AutoSponge
Copy link

@sonewman I think we have the fix for this in SystemJS now. It will guard against this case by injecting an @empty module (ref above).

@calvinmetcalf
Copy link
Contributor

closing this as it is browserifiable and if other build systems have issues we can open up issues for those

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants