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

Add clear timeout polyfill #451

Merged
merged 8 commits into from
Jun 13, 2016
Merged

Add clear timeout polyfill #451

merged 8 commits into from
Jun 13, 2016

Conversation

martyphee
Copy link
Contributor

@martyphee martyphee commented Jun 11, 2016

Added clearTimeout to execjs_timer_polyfills.


This change is Reviewable

@martyphee
Copy link
Contributor Author

@justin808 I didn't see any current tests to add this to. We're currently running 5.2. I have to see about upgrading to 6.x

@xiaopow
Copy link

xiaopow commented Jun 11, 2016

@martyphee Anyway I can start using this code for my app? I haven't found any other work around. Would you know how to write to the beginning of the webpack bundled file through settings in webpack config?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 82.305% when pulling 2318c9a on martyphee:add-clearTimeout-polyfill into 3fe05f9 on shakacode:master.

@martyphee
Copy link
Contributor Author

I have a work around for anyone interested.

cd client
rm -rf node_modules
npm install
npm shrinkwrap

Edit shrinkwrap.json. Update process like this.

"process": {
    "version": "0.11.3",
    "from": "process@>=0.11.0 <0.12.0",
    "resolved": "http://registry.npmjs.org/process/-/process-0.11.3.tgz"
}

@martyphee
Copy link
Contributor Author

@justin808 I have a PR out there to polyFill and I have a PR for our codebase to bump it to 6.x of react_on_rails.

@justin808
Copy link
Member

I'm reviewing. Tests would be a bonus.

Previously, martyphee (Marty Phee) wrote…

@justin808 I have a PR out there to polyFill and I have a PR for our codebase to bump it to 6.x of react_on_rails.


Review status: 0 of 1 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@justin808
Copy link
Member

Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion.


lib/react_on_rails/server_rendering_pool/exec.rb, line 137 [r1] (raw file):

function clearTimeout() {
  #{undefined_for_exec_js_logging('setTimeout')}

should be 'clearTimeout'


Comments from Reviewable

@justin808
Copy link
Member

One comment.

Previously, justin808 (Justin Gordon) wrote…

I'm reviewing. Tests would be a bonus.


Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@justin808
Copy link
Member

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@martyphee
Copy link
Contributor Author

Just pushed update. Sorry about that.

On Sat, Jun 11, 2016 at 4:11 PM, Justin Gordon [email protected]
wrote:

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved

discussion.

Comments from Reviewable
https://reviewable.io:443/reviews/shakacode/react_on_rails/451


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#451 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAuEtqfB2_VjEKXP7jHgeSWHM29RCPx8ks5qKyRwgaJpZM4IzlLB
.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 82.305% when pulling 5c97165 on martyphee:add-clearTimeout-polyfill into 3fe05f9 on shakacode:master.

@justin808
Copy link
Member

@martyphee Please update the Changelog.md per the examples, and I'll merge and release 6.0.4.

Previously, coveralls wrote…

Coverage Status

Coverage remained the same at 82.305% when pulling 5c97165 on martyphee:add-clearTimeout-polyfill into 3fe05f9 on shakacode:master.


Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@martyphee
Copy link
Contributor Author

Done

@coveralls
Copy link

Coverage Status

Coverage remained the same at 82.305% when pulling 173176e on martyphee:add-clearTimeout-polyfill into 3fe05f9 on shakacode:master.

@justin808
Copy link
Member

Please see the comment and squash to one commit and I'll release. Thanks!

Previously, coveralls wrote…

Coverage Status

Coverage remained the same at 82.305% when pulling 173176e on martyphee:add-clearTimeout-polyfill into 3fe05f9 on shakacode:master.


Reviewed 1 of 1 files at r3.
Review status: all files reviewed at latest revision, 2 unresolved discussions.


CHANGELOG.md, line 8 [r3] (raw file):

## [Unreleased]

## [6.0.4]

See the bottom of the file for where this goes.


Comments from Reviewable

@coveralls
Copy link

Coverage Status

Coverage remained the same at 82.305% when pulling ec2e83c on martyphee:add-clearTimeout-polyfill into 3fe05f9 on shakacode:master.

@justin808
Copy link
Member

:lgtm: Thanks @martyphee

Previously, coveralls wrote…

Coverage Status

Coverage remained the same at 82.305% when pulling ec2e83c on martyphee:add-clearTimeout-polyfill into 3fe05f9 on shakacode:master.


Review status: 1 of 2 files reviewed at latest revision, 2 unresolved discussions.


Comments from Reviewable

@justin808
Copy link
Member

Reviewed 1 of 1 files at r4.
Review status: all files reviewed at latest revision, 2 unresolved discussions.


Comments from Reviewable

@justin808 justin808 merged commit e13fe9b into shakacode:master Jun 13, 2016
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.

4 participants