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

Add Type Safety Guards to turbo/fetch_requests #403

Merged
merged 2 commits into from Nov 20, 2022

Conversation

seanpdoyle
Copy link
Contributor

@seanpdoyle seanpdoyle commented Nov 18, 2022

Without this change, GET <form> submissions without a Fetch Options { body: } raise the following:

Uncaught TypeError: s.body is null
    <anonymous> fetch_requests.js:10
    w turbo.es2017-esm.js:351
    requestStarted turbo.es2017-esm.js:770
    perform turbo.es2017-esm.js:520
    start turbo.es2017-esm.js:744
    formSubmitted turbo.es2017-esm.js:3369

Through the exercise of attempting to port the turbo/fetch_requests module to
TypeScript
, we've identified some potential edge cases in the algorithm that determines a request's HTTP method.

Even if the migration to TypeScript doesn't come to fruition for some time, those edge cases should be addressed sooner rather than later.

This commit adds more "type safety" motivated conditionals and guards to make sure that values are present before overriding them.

@seanpdoyle seanpdoyle force-pushed the fetch-request-improvements branch 2 times, most recently from c804ecd to 6d87887 Compare November 19, 2022 01:09
@seanpdoyle
Copy link
Contributor Author

cc: @dhh

@seanpdoyle seanpdoyle force-pushed the fetch-request-improvements branch 7 times, most recently from 20001fd to 567edd3 Compare November 19, 2022 18:10
Without this change, GET `<form>` submissions without a Fetch Options `{
body: }` raise the following:

```
Uncaught TypeError: s.body is null
    <anonymous> fetch_requests.js:10
    w turbo.es2017-esm.js:351
    requestStarted turbo.es2017-esm.js:770
    perform turbo.es2017-esm.js:520
    start turbo.es2017-esm.js:744
    formSubmitted turbo.es2017-esm.js:3369
```

Through the exercise of attempting to [port the `turbo/fetch_requests`
module to
TypeScript](hotwired#392 (comment)),
we've identified some potential edge cases in the algorithm that
determines a request's HTTP method.

Even if the migration to TypeScript doesn't come to fruition for some
time, those edge cases should be addressed sooner rather than later.

This commit adds more "type safety" motivated conditionals and guards to
make sure that values are present before overriding them.
@seanpdoyle seanpdoyle force-pushed the fetch-request-improvements branch 6 times, most recently from 27d4e14 to efe8f43 Compare November 19, 2022 19:20
Replace looping mechanisms with Capybara assertion for the presence of a
`<turbo-cable-stream-source>` element.
@dhh dhh merged commit cf69d2a into hotwired:main Nov 20, 2022
@kzkn
Copy link

kzkn commented Nov 23, 2022

(edit) I was commenting in the wrong place 😅 Moved to here: #392 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants