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 e.set to accept an array of values and a type + prevent arrays from being spread into e.set #513

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jaclarke
Copy link
Member

@jaclarke jaclarke commented Dec 20, 2022

Current behaviour of e.set is buggy when spreading an array of values into it, since at runtime it's not possible to tell if an array was spread or the values were passed statically. This causes a mis-match in the cardinality determined statically by typescript and at runtime. If the array happens to be empty, it also causes a runtime error, as the type of the empty set cannot be determined. Eg:

const strs = ['abc']; // has type string[]

e.set(...strs); // ts infers cardinality as Many since 'strs' could have any length

e.set('abc'); // ts infers cardinality as One

// at runtime both above appear the same, so cardinality cannot be correctly determined for both

This PR prevents arrays from being spread into e.set (unless it's a tuple, eg. const strs = ['abc'] as const), and instead allows the array to be passed directly, along with a type expr, so the empty array case can be handled. The addition of a type arg also means it's no longer necessary to wrap each item of the array for non js primitive types:

const uuids: string[] = [/* some array of uuid strings */];

e.set(e.uuid, uuids);

@zerosym zerosym mentioned this pull request May 15, 2023
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.

1 participant