3 ms·

To reinvent it.

by bnetd 2y ago

To reinvent it.

benatkin 2y ago
People tried to argue that in the PR, suggesting to change variable names. This one: https://github.com/hotwired/turbo/pull/971/files#r1317386731 https://github.com/hotwired/turbo/pull/971/files#r1317386731 > function buildFormData(formElement: HTMLFormElement, submitter?: HTMLElement): FormData { > It might be useful to know that submitter is optional argument here. > How would one express that? The variable now needs to be renamed? And two lines later: > const name = submitter?.getAttribute("name") It's already easy to see that submitter is optional.
- lelandbatey 2y ago
  Only easy to see that it's optional if you read the source code vs. looking at the auto definition in your IDE/in autogenerated documentation.
- seanwilson 2y ago
  > It's already easy to see that submitter is optional. Why force yourself to read inside function implementations like this for every function you might use when fool-proof automated assistance is right there? What if the function was tens of lines long? What if the optional value only appeared inside a chain of 5 nested functions? How would you easily fix/update your code if the value isn't optional now but got changed to be optional in the future? Manual effort like this is error-prone, not scalable, and a huge distraction. Why would avoiding type annotations be worth this?
  - benatkin 2y ago
    Because this is an internal function in a _library_, and you should be willing to look inside it if you're changing a call to it. Here's where whether or not it's defined can be traced to: https://github.com/hotwired/turbo/blob/41c074ff113a8882aadbc2758b76860efbcfbf01/src/observers/form_submit_observer.js#L30 https://github.com/hotwired/turbo/blob/41c074ff113a8882aadbc... Maybe the types should have been written differently, to indicate that it's the type of the submitter property in a FormEvent. That's another cost of using TypeScript, is trying to get the types right. TMTOWTDI.
    - seanwilson 2y ago
      > Because this is an internal function in a _library_ Why does that matter? Error-prone approaches create buggy code and we don't want buggy code, whether it's a library or something else. Libraries can be huge and complex, and arguably should have more robust edge case checking than regular code.
- knallfrosch 2y ago
  You're only able to see it by reading the function body.

To reinvent it.

by bnetd 2y ago

To reinvent it.

People tried to argue that in the PR, suggesting to change variable names. This one: https://github.com/hotwired/turbo/pull/971/files#r1317386731 https://github.com/hotwired/turbo/pull/971/files#r1317386731 > function buildFormData(formElement: HTMLFormElement, submitter?: HTMLElement): FormData { > It might be useful to know that submitter is optional argument here. > How would one express that? The variable now needs to be renamed? And two lines later: > const name = submitter?.getAttribute("name") It's already easy to see that submitter is optional.

lelandbatey 2y ago
Only easy to see that it's optional if you read the source code vs. looking at the auto definition in your IDE/in autogenerated documentation.
seanwilson 2y ago
> It's already easy to see that submitter is optional. Why force yourself to read inside function implementations like this for every function you might use when fool-proof automated assistance is right there? What if the function was tens of lines long? What if the optional value only appeared inside a chain of 5 nested functions? How would you easily fix/update your code if the value isn't optional now but got changed to be optional in the future? Manual effort like this is error-prone, not scalable, and a huge distraction. Why would avoiding type annotations be worth this?
- benatkin 2y ago
  Because this is an internal function in a _library_, and you should be willing to look inside it if you're changing a call to it. Here's where whether or not it's defined can be traced to: https://github.com/hotwired/turbo/blob/41c074ff113a8882aadbc2758b76860efbcfbf01/src/observers/form_submit_observer.js#L30 https://github.com/hotwired/turbo/blob/41c074ff113a8882aadbc... Maybe the types should have been written differently, to indicate that it's the type of the submitter property in a FormEvent. That's another cost of using TypeScript, is trying to get the types right. TMTOWTDI.
  - seanwilson 2y ago
    > Because this is an internal function in a _library_ Why does that matter? Error-prone approaches create buggy code and we don't want buggy code, whether it's a library or something else. Libraries can be huge and complex, and arguably should have more robust edge case checking than regular code.
knallfrosch 2y ago
You're only able to see it by reading the function body.