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

don't check mapFn length #730

Closed
wants to merge 1 commit into from

Conversation

AlexErrant
Copy link
Contributor

In the course of writing <SetEntries>, I did a bit of digging on something I found confusing, resulting in this PR.

I'm not sure why we need to check the parameter count of the mapFn:

mapFn.length < 3
? key =>
(mapFn as (key: K, v: Accessor<V>) => JSX.Element)(
key,
() => (props.of as Map<K, V>).get(key)!,
)
: (key, i) => mapFn(key, () => (props.of as Map<K, V>).get(key)!, i),
"fallback" in props ? { fallback: () => props.fallback } : undefined,

Without types for easier reading:

mapFn.length < 3
    ? (key   ) => mapFn(key, () => props.of.get(key))
    : (key, i) => mapFn(key, () => props.of.get(key), i),

The mapFn is user-defined and may take 2 or 3 parameters. However, I believe that this is irrelevant and can be simplified to

(key, i) => mapFn(key, () => props.of.get(key), i),

You can see that in the following code, if you pass 3 arguments to a function that has 2 parameters, the 3rd argument is ignored:

let xs = ["a", "b", "c"]

const userFn2 = (key: string, val: string) => {
    return [key, val]
}

const userFn3 = (key: string, val: string, i: number) => {
    return [key, val, i]
}

console.log("userFn2       ", xs.map(x      => userFn2(x, x)))
console.log("userFn3       ", xs.map((x, i) => userFn3(x, x, i)))
console.log("userFn2 with 3", xs.map((x, i) => userFn2(x, x, i)))
//[LOG]: "userFn2       ",  [["a", "a"], ["b", "b"], ["c", "c"]] 
//[LOG]: "userFn3       ",  [["a", "a", 0], ["b", "b", 1], ["c", "c", 2]] 
//[LOG]: "userFn2 with 3",  [["a", "a"], ["b", "b"], ["c", "c"]]

Playground.

This demos what happens when the user gives us a fn with 2 parameters, but solid-primitives gives it 3 arguments.

Relevant StackOverflow.

Perhaps I'm missing something obvious - I'm not super experienced with JS 😅

Copy link

changeset-bot bot commented Jan 5, 2025

⚠️ No Changeset found

Latest commit: 5b04714

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@thetarnav
Copy link
Member

Check the source for mapArray
it doesn't create signals for index if the user does not use it

@AlexErrant AlexErrant closed this Jan 5, 2025
@AlexErrant AlexErrant deleted the mapFn_length branch January 5, 2025 21:11
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.

2 participants