Skip to content
This repository has been archived by the owner. It is now read-only.

Bug in computed properties implementation in Chrome < 52 #54

Closed
ahutchings opened this issue Nov 30, 2016 · 8 comments

Comments

Projects
None yet
6 participants
@ahutchings
Copy link

commented Nov 30, 2016

In our app, we were experiencing a crash in certain scenarios when using computed properties in Chrome < 52. This bug has been fixed in Chrome 52 and above.

https://bugs.chromium.org/p/v8/issues/detail?id=5033
https://bugs.chromium.org/p/chromium/issues/detail?id=625868

Given that, would you accept a pull request to bump the required Chrome version to 52 for transform-es2015-computed-properties? I suppose Opera would have been affected as well. Would this need to be authored as an override in the build-data.js script?

cc @jayfunk

@hzoo

This comment has been minimized.

Copy link
Member

commented Nov 30, 2016

II think we should figure out a way to override it since everytime we run the build script it would change it back to the old value?

I think it depends on the case that it happens as well? Not sure

@chicoxyzzy

This comment has been minimized.

Copy link
Member

commented Nov 30, 2016

Probably we can add a test to compat-table. But I'm afraid that there is no even test262 test for it.

@ljharb

This comment has been minimized.

Copy link
Member

commented Dec 2, 2016

We should add a test to both - test262 takes PRs as well :-)

@fhinkel

This comment has been minimized.

Copy link
Member

commented Dec 4, 2016

V8 has a regression test for this case (https://github.com/v8/v8/blob/master/test/mjsunit/regress/regress-5033.js). I'm not sure how much sense this makes to add a test to test262, since the issue only happened with the optimizing compiler.

@stale stale bot added the i: stale label Jun 13, 2017

@stale

This comment has been minimized.

Copy link

commented Jun 13, 2017

This issue has been automatically marked as stale because it has not had recent activity 😴. It will be closed if no further activity occurs. Thank you for your contributions 👌!

@ljharb

This comment has been minimized.

Copy link
Member

commented Jun 13, 2017

Um, why are we using this bot? "Stale" issues should remain open indefinitely to ensure discoverability.

@ljharb

This comment has been minimized.

Copy link
Member

commented Jun 13, 2017

If there's no way to reliably check for this without %OptimizeFunctionOnNextCall, then definitely no test could be written for it.

That said, if something's crashing in Chrome < 52, it's a bug, and it needs fixing.

If bumping the required version for the transform to v52 is a simple fix for this problem - especially since few users will run into this anyways - then that seems like the right path.

@babel-bot

This comment has been minimized.

Copy link

commented Oct 30, 2017

This issue has been moved to babel/babel#6641.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.