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

WIP fix(patterns,exo)!: args, splitArray, and splitRecord now default to … #1764

Closed
wants to merge 1 commit into from

Conversation

erights
Copy link
Contributor

@erights erights commented Sep 11, 2023

Experiment re surprise that method guards with no declared rest by default accept extra args. This PR represents the most extreme and least compat "fix" to this surprise -- changing the semantics of splitArray and splitRecord so they also reject rest parts by default, i.e., if no rest argument is provided. Because of the way method guards delegate to splitArray, this takes care of the method guard "problem" as well.

Other options:

  • Fix only method guards, so that they reject extra args by default, by changing how it delegates to splitArray. This "solution" would not change the semantics of splitArray at all.
  • Grandfather in exactly the current tolerant behavior of all three: method guards, splitArray, and splitRecord. This tolerant behavior of methodGuards can be rationalized as reflecting the tolerant behavior of JS positional function parameters.
  • Preserve the tolerant behavior, but not pass the values of the extra args through to the protected raw behavior method. This would preserve the input validation intent of method guards while still accommodating API growth and thus version slippage. (suggested by @turadg )

@erights erights self-assigned this Sep 11, 2023
@mhofman
Copy link
Contributor

mhofman commented Sep 12, 2023

  • Preserve the tolerant behavior, but not pass the values of the extra args through to the protected raw behavior method. This would preserve the input validation intent of method guards while still accommodating API growth and thus version slippage. (suggested by @turadg )

I'm in favor of this approach as it's the one I had initially suggested as the behavior I'd expect.

@erights
Copy link
Contributor Author

erights commented Sep 13, 2023

Closing in favor of #1765

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