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

FakeDOMStringList should not support array methods #66

Open
shadow-light opened this issue Sep 18, 2021 · 4 comments
Open

FakeDOMStringList should not support array methods #66

shadow-light opened this issue Sep 18, 2021 · 4 comments

Comments

@shadow-light
Copy link

shadow-light commented Sep 18, 2021

FakeDOMStringList seems to be extending array, but a number of array methods aren't available. Such that tests can pass and then result in failure in production.

I had a migration that executed transaction.objectStoreNames.includes('...') and didn't cause any issues, but then failed in production as should have been transaction.objectStoreNames.contains('...'). Didn't pick up on it with types due to a bug in idb

@dumbmatter
Copy link
Owner

This is a good point.

I think FakeDOMStringList does need to subclass/extend the built-in Array, because otherwise I don't know how you'd get various proper behaviors like destructuring. Please correct me if I'm wrong.

But it should either delete the Array-specific methods from the class or if that's not possible manually override them to throw errors. I think that should be possible, at least.

I was planning on doing a new release soon, I will try to include this.

@shadow-light
Copy link
Author

That sounds good, thanks for looking into it

@dumbmatter
Copy link
Owner

Turns out the solution I came up with in 7879a24 causes some problems when used with Dexie. I'm not sure why. But I guess it'll be safer to not do this, unless someone can help me figure out a better way.

@dumbmatter
Copy link
Owner

[].slice.call(arrayLike) is the problem, that is supposed to turn a DOMStringList into an array, but was not actually restoring the deleted methods.

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

No branches or pull requests

2 participants