process.hrtime change breaks browser support

[karl]

To recreate:

• Go to https://jsfiddle.net/yexyLdj7/ - This uses the latest version of lolex-src.js from master.
• Run the fiddle.

What happens:

• The browser throws the error: lolex-src.js:623 Uncaught ReferenceError: global is not defined

What should happen:

• No error!

It looks like it is caused by this line: https://github.com/sinonjs/lolex/blob/master/src/lolex-src.js#L392

I'm not sure how to write a test that shows this issue!

[mantoni]

Well, you're testing against the source file. It is meant to be browserified which will provide the process object. To create a new lolex.js in the root of the project, you have to npm run bundle.

[karl]

Ah, interesting.

I see the same problem (the error where process is not defined) when I try and use the latest commit of lolex in my work project.

The project is built using Webpack, which I believe will use the source version of lolex as that is what is specified in package.json. It doesn't look like Webpack is providing the process object, which leads lolex throwing an error.

Does that make sense? I don't have time at the moment to check that all my assumptions are correct I'm afraid!

[mantoni]

Yes, that sounds about right. The current master build runs the tests in real browsers via SauceLabs. Since the build is green I think it's working fine, but I can also not check myself at the moment.

Closing for now. If we find this isn't working for some reason, we can reopen.

[karl]

@mantoni: I've created a minimal reproduction of this issue in the following repository:

https://github.com/karl/lolex-webpack-error

It seems my assumptions are correct and the source version of lolex is used when requiring it from the npm package. The source version expects the process object, but it isn't defined in the browser and Webpack doesn't provide a dummy version.

This doesn't explain why the tests pass on SauceLabs though! That might require some more investigation from your side.

[fatso83]

This doesn't explain why the tests pass on SauceLabs though!

Actually it does, @karl :-) As @mantoni said

it is meant to be browserified which will provide the process object

The tests run using mochify, which as the name suggests is "TDD with Browserify, Mocha, PhantomJS and WebDriver". So the tests are bundled with Browserify before being run in the browsers, which explains why process is present.

[karl]

Ah, that explains it!

Do you have any ideas on the best way to fix the Webpack error?

Perhaps package.json could point to the browserified version of lolex. Would this ensure that global.process is always defined?

[fatso83]

@karl I googled, and it seems like I do now :smile_cat: You can use the DefinePlugin in Webpack to fix it. See this somewhat related answer

[karl]

Do you think it would be worth fixing this in the Lolex codebase (by checking for existence of process before attempting to use it)?

I worry that requiring anyone using Webpack to use the DefinePlugin in order to Lolex is an unnecessary burden. Especially as the current 1.4.0 release works fine without needing the DefinePlugin.

[mantoni]

@karl I think the fix is to make Webpack load the right file. We decided to use browserify, so it makes little sense to implement a workaround in the code in case browserify isn't used. If we want this to be checked by Lolex, we should also opt out of shimming the process object entirely so that the behavior is consistent in all environments.

[karl]

@mantoni That sounds sensible!

I can have a look at making a PR that has package.json point to ./lolex.js (as this has already been though browserify).

[mantoni]

Ha! It's actually a lot simpler than I thought: Lolex does not have a browser field yet. We should add "browser": "./lolex.js" to the package.json to comply with the pacakge-browser-field-spec. Webpack inspects this field and will do the right thing.

[mantoni]

Although this means that other projects like Sinon that use Lolex and are browserified themselves would load the pre-bundled version instead of the actual source ... Not so great then :disappointed:

[fatso83]

@mantoni : don't understand why this won't work.

• If Sinon is used through Node, then it will require packages normally, using the main field.
• If Sinon is being browserified then it will use the pre-packaged version of lolex. This should still work, right?

[mantoni]

@fatso83 Yes, I think it would work, but it's the wrong approach. Imagine we had some common utilities that are used in Lolex and in Sinon. If they go into a separate module, it would be included twice in the final bundle. It would still work, but there can be unexpected effects if modules are loaded more than once.

However, Webpack also looks at a specific "webpack" field. We could add this field to the package.json instead.

[karl]

I've been tinkering around, and decided to try using the lolex.js bundle generated by Browserify in my lolex-webpack-error repository.

Surprisingly it still fails with an error that process is not defined. It seems that Browserify doesn't provide the process object like we thought (unless I have got something wrong!).

You can see the change I made to the test repo here: https://github.com/karl/lolex-webpack-error/commit/37ced636235879e447431106480afc354a163e6c

[mantoni]

Hm, that is interesting. The only explanation that I have is that Browserify does not shim process because it’s referred to by global.process instead of just process as one usually would in the node context. Mochify internally depends on the process object for the exit code detection, so that would explain why it works in the test cases. @karl Can you try removing the global. part and check again?

[karl]

I have a branch that refers to process as a global:

https://github.com/karl/lolex/tree/spike-browserify-process

It works, but including a shim for process seems excessive given all we do with it is check for the hrtime function.

I've created a second branch that checks for the existence of process before using it. With this change Lolex now works in browsers and works when packaged with Webpack.

https://github.com/sinonjs/lolex/pull/53