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

Convert JavaScript To TypeScript #392

Closed
wants to merge 14 commits into from
Closed

Convert JavaScript To TypeScript #392

wants to merge 14 commits into from

Conversation

kzkn
Copy link

@kzkn kzkn commented Oct 8, 2022

For fix #18

}

export async function createConsumer() {
const { createConsumer } = await import(/* webpackChunkName: "actioncable" */ "@rails/actioncable/src")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could't set "strict": true in tsconfig.json because I couldn't resolve the type error on this line.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have fixed the type error.

@seanpdoyle
Copy link
Contributor

seanpdoyle commented Nov 18, 2022

@kzkn thank you for taking this one on!

As the original author of app/javascript/turbo/fetch_requests.ts, I thought it'd be good to pull this down locally and test out the types. How do you feel about making some of the following changes?

diff --git a/app/javascript/turbo/fetch_requests.ts b/app/javascript/turbo/fetch_requests.ts
index a62e0a4..8e5c027 100644
--- a/app/javascript/turbo/fetch_requests.ts
+++ b/app/javascript/turbo/fetch_requests.ts
@@ -1,36 +1,54 @@
-import type { TurboBeforeFetchRequestEvent, TurboSubmitStartEvent } from '@hotwired/turbo'
+import type { TurboBeforeFetchRequestEvent, TurboSubmitStartEvent } from "@hotwired/turbo"
 
-export function encodeMethodIntoRequestBody(event: Event) {
-  let ev = event as TurboBeforeFetchRequestEvent
-  if (ev.target instanceof HTMLFormElement) {
-    const { target: form, detail: { fetchOptions } } = ev
+export function encodeMethodIntoRequestBody(event: TurboBeforeFetchRequestEvent) {
+  if (event.target instanceof HTMLFormElement) {
+    const { target: form, detail: { fetchOptions } } = event
 
-    form.addEventListener("turbo:submit-start", ev => {
-      const { detail: { formSubmission: { submitter } } } = ev as TurboSubmitStartEvent
-      const method = detectSubmitMethod(form, submitter, fetchOptions) || ""
+    form.addEventListener("turbo:submit-start", <EventListener>(({ detail: { formSubmission: { submitter } } }: TurboSubmitStartEvent) => {
+      const body = isBodyInit(fetchOptions.body) ? fetchOptions.body : new URLSearchParams()
+      const method = determineFetchMethod(submitter, body, form)
 
       if (!/get/i.test(method)) {
-        const body = fetchOptions.body as FormData | undefined
         if (/post/i.test(method)) {
-          body?.delete("_method")
+          body.delete("_method")
         } else {
-          body?.set("_method", method)
+          body.set("_method", method)
         }
 
         fetchOptions.method = "post"
       }
-    }, { once: true })
+    }), { once: true })
   }
 }
 
-function detectSubmitMethod(form: HTMLElement, submitter: HTMLElement | undefined, fetchOptions: RequestInit): string | null {
-  const _submitter = submitter as HTMLInputElement | HTMLButtonElement | undefined
-  const body = fetchOptions.body as FormData | undefined
-  if (_submitter && _submitter.formMethod) {
-    return _submitter.formMethod
-  } else if (body?.get("_method")) {
-    return body.get("_method") as string | null
+function determineFetchMethod(submitter: HTMLElement | undefined, body: FetchBodyData, form: HTMLFormElement): string {
+  const formMethod = determineFormMethod(submitter)
+  const overrideMethod = body.get("_method")
+  const method = form.getAttribute("method") || "get"
+
+  if (typeof formMethod == "string") {
+    return formMethod
+  } else if (typeof overrideMethod == "string") {
+    return overrideMethod
+  } else {
+    return method
+  }
+}
+
+function determineFormMethod(submitter: HTMLElement | undefined): string | null {
+  if (submitter instanceof HTMLButtonElement || submitter instanceof HTMLInputElement) {
+    if (submitter.hasAttribute("formmethod")) {
+      return submitter.formMethod
+    } else {
+      return null
+    }
   } else {
-    return form.getAttribute("method")
+    return null
   }
 }
+
+type FetchBodyData = FormData | URLSearchParams
+
+function isBodyInit(body: BodyInit | null | undefined): body is FetchBodyData {
+  return body instanceof FormData || body instanceof URLSearchParams
+}
diff --git a/app/javascript/turbo/index.ts b/app/javascript/turbo/index.ts
index 7a75be5..e280c5b 100644
--- a/app/javascript/turbo/index.ts
+++ b/app/javascript/turbo/index.ts
@@ -8,4 +8,4 @@ export { cable }
 
 import { encodeMethodIntoRequestBody } from "./fetch_requests"
 
-addEventListener("turbo:before-fetch-request", encodeMethodIntoRequestBody)
+addEventListener("turbo:before-fetch-request", <EventListener>encodeMethodIntoRequestBody)

seanpdoyle added a commit to seanpdoyle/turbo-rails that referenced this pull request Nov 18, 2022
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 added a commit to seanpdoyle/turbo-rails that referenced this pull request Nov 18, 2022
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 added a commit to seanpdoyle/turbo-rails that referenced this pull request Nov 18, 2022
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 added a commit to seanpdoyle/turbo-rails that referenced this pull request Nov 19, 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](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 added a commit to seanpdoyle/turbo-rails that referenced this pull request Nov 19, 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](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 added a commit to seanpdoyle/turbo-rails that referenced this pull request Nov 19, 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](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 added a commit to seanpdoyle/turbo-rails that referenced this pull request Nov 19, 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](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 added a commit to seanpdoyle/turbo-rails that referenced this pull request Nov 19, 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](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 added a commit to seanpdoyle/turbo-rails that referenced this pull request Nov 19, 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](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 added a commit to seanpdoyle/turbo-rails that referenced this pull request Nov 19, 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](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 added a commit to seanpdoyle/turbo-rails that referenced this pull request Nov 19, 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](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 added a commit to seanpdoyle/turbo-rails that referenced this pull request Nov 19, 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](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.
dhh pushed a commit that referenced this pull request Nov 20, 2022
* Add Type Safety Guards to `turbo/fetch_requests`

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](#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.

* Resolve Turbo Stream test flakiness

Replace looping mechanisms with Capybara assertion for the presence of a
`<turbo-cable-stream-source>` element.
Copy link
Member

@marcoroth marcoroth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for starting this one @kzkn!

This looks good to me so far, I've left two small improvements, but they are non-blocking.

app/javascript/turbo/cable.ts Outdated Show resolved Hide resolved
Comment on lines 3 to 8
export function encodeMethodIntoRequestBody(event: Event) {
let ev = event as TurboBeforeFetchRequestEvent
if (ev.target instanceof HTMLFormElement) {
const { target: form, detail: { fetchOptions } } = ev

form.addEventListener("turbo:submit-start", ev => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To add what @seanpdoyle mentioned in the previous comment: I've opened hotwired/turbo#800 to help with this.

With that you would be able to do something like:

Suggested change
export function encodeMethodIntoRequestBody(event: Event) {
let ev = event as TurboBeforeFetchRequestEvent
if (ev.target instanceof HTMLFormElement) {
const { target: form, detail: { fetchOptions } } = ev
form.addEventListener("turbo:submit-start", ev => {
export function encodeMethodIntoRequestBody(event: TurboBeforeFetchRequestEvent) {
if (event.target instanceof HTMLFormElement) {
const { target: form, detail: { fetchOptions } } = event
form.addEventListener("turbo:submit-start", (ev: TurboSubmitStartEvent) => {

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good to me. I look forward to seeing the pullreq merged 😀

kzkn added a commit to kzkn/turbo-rails that referenced this pull request Nov 23, 2022
kzkn added a commit to kzkn/turbo-rails that referenced this pull request Nov 23, 2022
@kzkn
Copy link
Author

kzkn commented Nov 23, 2022

@seanpdoyle
Thanks for the comment.
I had a hard time sorting this part out. Your code looks better than mine 😄
I rewrote it based on your comments and the code written in #403.

- `import('@rails/actioncable/src')` -> `import('@rails/actioncable')` for fix the type error
- `removeComments: false` for keep `webpackChunkName` comment
@kzkn
Copy link
Author

kzkn commented Jan 6, 2023

I have rebased to follow #410

@brunoprietog
Copy link
Contributor

Since Turbo switched to JavaScript, I think we can close this

@kzkn kzkn closed this Nov 29, 2023
@kzkn kzkn deleted the typescript branch November 29, 2023 11:03
atosbucket added a commit to atosbucket/turbo-rails that referenced this pull request Apr 11, 2024
* Add Type Safety Guards to `turbo/fetch_requests`

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/turbo-rails#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.

* Resolve Turbo Stream test flakiness

Replace looping mechanisms with Capybara assertion for the presence of a
`<turbo-cable-stream-source>` element.
newheaven918 added a commit to newheaven918/turbo that referenced this pull request Apr 26, 2024
* Add Type Safety Guards to `turbo/fetch_requests`

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/turbo-rails#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.

* Resolve Turbo Stream test flakiness

Replace looping mechanisms with Capybara assertion for the presence of a
`<turbo-cable-stream-source>` element.
luisjose1996 added a commit to luisjose1996/Turbo-Rails that referenced this pull request May 10, 2024
* Add Type Safety Guards to `turbo/fetch_requests`

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/turbo-rails#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.

* Resolve Turbo Stream test flakiness

Replace looping mechanisms with Capybara assertion for the presence of a
`<turbo-cable-stream-source>` element.
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.

TS7016: Could not find a declaration file for module '@hotwired/turbo-rails'
4 participants