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

fixed: setImmediate is not defined #24

Closed
wants to merge 1 commit into from
Closed

Conversation

taoqf
Copy link
Contributor

@taoqf taoqf commented Jul 24, 2017

When using module mqtt and webpack, variant setImmediate maybe defined.

@taoqf
Copy link
Contributor Author

taoqf commented Jul 25, 2017

#25

@taoqf
Copy link
Contributor Author

taoqf commented Jul 27, 2017

@jryans Could you please merge this pr if appropriate? I'm working on a project need this very much. Thanks.

Copy link
Member

@jryans jryans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rework this to avoid extra whitespace changes.

main.js Outdated
};

// setimmediate attaches itself to the global object
require("setimmediate");
exports.setImmediate = setImmediate;
exports.clearImmediate = clearImmediate;
var global = require('global');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use double quotes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

main.js Outdated
exports.setImmediate = setImmediate;
exports.clearImmediate = clearImmediate;
var global = require('global');
exports.setImmediate = global.setImmediate || global.setTimeout;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need setTimeout here? Is setImmediate not actually attaching to the global at all?

If so, it sounds like something needs to be fixed in the setimmediate library first. It doesn't seem to return anything on require, so it seems like it needs to be convinced to attach.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the global thing, anyway, i don't use it.

@jryans
Copy link
Member

jryans commented Jul 28, 2017

Looks like whitespace changes are still an issue, please use 2 space indents and revert whitespace for unchanged lines.

@taoqf
Copy link
Contributor Author

taoqf commented Jul 31, 2017

@jryans could this pr be merged?

@taoqf
Copy link
Contributor Author

taoqf commented Jul 31, 2017

I do need this very much, would you please spend some time on this pr @jryans

@jryans
Copy link
Member

jryans commented Jul 31, 2017

Thanks for the PR! I merged a slightly tweaked version just now.

@jryans jryans closed this Jul 31, 2017
@jryans
Copy link
Member

jryans commented Jul 31, 2017

I have published 2.0.3 which should contain the fix.

@taoqf
Copy link
Contributor Author

taoqf commented Jul 31, 2017

Thanks

@jryans
Copy link
Member

jryans commented Aug 14, 2017

Unfortunately, I had to revert this in 2.0.4 for breaking other Webpack users. We'll need better testing if we want to try this again.

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

Successfully merging this pull request may close these issues.

2 participants