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

Bug: DevTools calls arbitrary generators which may be stateful #19726

Closed
gaearon opened this issue Aug 30, 2020 · 11 comments · Fixed by #19831
Closed

Bug: DevTools calls arbitrary generators which may be stateful #19726

gaearon opened this issue Aug 30, 2020 · 11 comments · Fixed by #19831

Comments

@gaearon
Copy link
Collaborator

gaearon commented Aug 30, 2020

function foo*() {
  yield 1;
  yield 2;
}

let gen = foo()

Currently if you put gen into state or props and then open this component in DevTools, it will consume that generator while trying to format it. So gen.next() will give you { done: true } next time you call it.

This happens here:

case 'iterator':
const name = data.constructor.name;
if (showFormattedValue) {
// TRICKY
// Don't use [...spread] syntax for this purpose.
// This project uses @babel/plugin-transform-spread in "loose" mode which only works with Array values.
// Other types (e.g. typed arrays, Sets) will not spread correctly.
const array = Array.from(data);

I think that maybe we should treat iterables differently if they return themselves as an iterator. Since that means they're likely stateful and it's not ok to iterate over them.

We detect iterables here (DevTools terminology is wrong btw, it should be iterable rather than iterator):

} else if (typeof data[Symbol.iterator] === 'function') {
return 'iterator';

I think maybe we could split this into iterable and opaque_iterable, and make sure none of the codepaths attempt to traverse opaque_iterable or pass it to something that would consume it (e.g. Array.from).

We could detect it based on data[Symbol.iterator]() === data — that clearly signals the iterable is its own iterator (which is the case for generators), and therefore it's not OK for DevTools to consume it.

Maybe some other heuristic could work. But overall, the goal is that Map and friends is still being iterated over, but an arbitrary generator is not.

@gaearon gaearon added Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug Component: Developer Tools Type: Bug good first issue and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Aug 30, 2020
@eps1lon
Copy link
Collaborator

eps1lon commented Aug 30, 2020

This is similar to #19660 where the same line of code causes devtools to crash with infinite generators.

I wasn't able to reproduce that devtools calling stateful generators was an issue though I definitely expected it to be a problem.

@todortotev
Copy link
Contributor

@gaearon I could give a shot on this!

@abelmark
Copy link

@gaearon I would love to give this a go.

@goanpixie
Copy link

@gaearon : I would like to address this bug

@eps1lon
Copy link
Collaborator

eps1lon commented Sep 1, 2020

@todortotev Go for it 👍 Please let us know if you no longer intend to work on it.

@todortotev
Copy link
Contributor

@todortotev Go for it 👍 Please let us know if you no longer intend to work on it.

On it!

@kambleaa007
Copy link

kambleaa007 commented Sep 3, 2020

just let us know branch you are putting your efforts, more guys would like to contribute, for it 👍
we should return opaque_iterable for data[Symbol.iterator]() === data

@omarsy
Copy link
Contributor

omarsy commented Sep 4, 2020

@eps1lon I agree with you both issues are related. if we choose to not extract generator. We’re not gonna have the issue anymore

@todortotev
Copy link
Contributor

Just a note, I'm nearly done! Sorry for being slow it just was my first attempt!

@engprodigy
Copy link

engprodigy commented Sep 12, 2020

@bvaughn
Copy link
Contributor

bvaughn commented Sep 22, 2020

This should be resolved by #19831 once it lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants