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

Update well-known-symbols #59

Open
novemberborn opened this issue Mar 22, 2020 · 3 comments
Open

Update well-known-symbols #59

novemberborn opened this issue Mar 22, 2020 · 3 comments
Assignees

Comments

@novemberborn
Copy link
Member

https://github.com/novemberborn/well-known-symbols/releases/tag/v3.0.0 adds Symbol.matchAll. I need to assess what that means for existing AVA snapshots that already include that symbol.

@novemberborn novemberborn self-assigned this Mar 22, 2020
@ninevra
Copy link
Contributor

ninevra commented Mar 31, 2021

@novemberborn sorry to nag, but might it make sense to resolve this before AVA@4.0.0?

It looks to me like declaring a symbol well-known might be a breaking change for AVA. It would cause existing snapshot assertions containing the symbol to start failing.

Have you considered dynamically determining whether a symbol is well-known, with something like:

const wellKnownSymbols = new Set(
  Reflect.ownKeys(Symbol)
    .filter(key => typeof Symbol[key] === 'symbol')
    .map(key => Symbol[key])
);

function isWellKnown(symbol) {
  return wellKnownSymbols.has(symbol);
}

@novemberborn
Copy link
Member Author

@novemberborn sorry to nag, but might it make sense to resolve this before AVA@4.0.0?

No worries, it's a good point!

Have you considered dynamically determining whether a symbol is well-known

That could be interesting. I need to see how these symbols are serialized and how compatibility across JS versions is dealt with. Perhaps the logic can be adjusted so we can add well-known symbols without causing compatibility issues.

@novemberborn
Copy link
Member Author

@ninevra I've decided not to tackle this for AVA 4. I'm hoping I can do some work on this package to modernize it and if need be that'll be AVA 5.

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