Check for existence of process object (fix for v3)

[th0r]

Fixes #412 for v3

Notes:

• Library was built using the following command: ./build/build.js 10.15.3
• Some unrelated files were changed because of the updated Babel version (updated runtime helpers)

[vweevers]

This is non-breaking for webpack users, right? But potentially breaking for browserify users.

[th0r]

This is non-breaking for webpack users, right?

Yes, it's a slightly modified version of the webpack's polyfill.

But potentially breaking for browserify users.

Well, potentially yes, but I don't think they will face any bugs. The only difference between these implementations is that the browserify's version drains queue synchronously on subsequent calls to nextTick.

[mcollina]

have you got a link to Browserify impl? I think Browserify version might be slightly faster and provide a better experience for users overall.

[th0r]

https://github.com/defunctzombie/node-process/blob/master/browser.js#L90-L154

[mcollina]

I prefer the browserify one, and more importantly this was designed to work with browserify first.

[th0r]

test/browser/test-stream2-unpipe-drain.js fails if I use browserify's version of nextTick.

I think it happens because test runner uses it's own version of process.nextTick polyfill (which comes from browserify) and readable-stream uses our own version so we end up with two different queues of async events.

webpack's version doesn't have this problem.

[th0r]

I've pushed a version with browserify's nextTick. If you want, you can try to fix this test - I don't have any more time which I can spend on it.

[mcollina]

Suggested change

	module.exports = nextTick;
	module.exports = process.nextTick;

[th0r]

In this case it doesn't pass this test.

[mcollina]

the best approach is to use module.exports.nextTick = process.nextTick pattern instead. Adding a wrapper is going to slow things down.

[th0r]

As I told you above it won't pass this test if I use your approach.
It happens because in this case readable-stream will still use original process.nextTick even after it will be replaced with the "fake" version.

[mcollina]

Oh, I understand now. You can do the following on the node side:

module.exports = process

While on the browser side:

module.exports = { nextTick }

Then, in code:

var wrap = require('./nextTick')
wrap.nextTick(function () {
  // this will work via lolex
})

[th0r]

Well, it's basically a polyfilling of a process object which I suggested here: #413 (comment)

[mcollina]

I think we were in agreement in there. I'll just not call it process.

[th0r]

But how do you want it to be called? wrap? Why not just process?

[mcollina]

We should not expect to be a full process object. In the browser, it won't be. It's an object with a nextTick property. we can call how we want "lightProcess" "fakeProcess" "nextTickWrap". This should enable that lolex support.

[mcollina]

have you got a link to Browserify impl? I think Browserify version might be slightly faster and provide a better experience for users overall.

[mcollina]

Can you please enable me to push on this PR? I have the test fixed.

[th0r]

@mcollina I've added you as collaborator to the forked repo if this is what you are asking for.

[th0r]

@mcollina any updates?

[vweevers]

I'm -1 on the nextTick change.

• Maintaining a polyfill as inline code is painful
• This change, intended for webpack users, will also affect browserify users. I would prefer that webpack 5 users include node-libs-browser in their configuration to get webpack 4 behavior.
• I'm worried that having a separate nextTick queue will introduce subtle, hard-to-debug race issues.