Typestrict #89
Typestrict #89
Conversation
jho406
force-pushed
the
typestrict
branch
5 times, most recently
from
782707c to
8132aef
Compare
| err.fetchArgs = fetchArgs | ||
| err.url = fetchArgs[0] | ||
| err.pageKey = urlToPageKey(fetchArgs[0]) |
There was a problem hiding this comment.
We're removing this as all are accessible through the developer's application_visit.js
| const hasPlaceholder = !!( | ||
| placeholderKey && getState().pages[placeholderKey] | ||
| ) |
There was a problem hiding this comment.
Typescript detected an edgecase where placeholder could be undefined.
| pageKey: key, //TODO remove this | ||
| savedAt: Date.now() |
There was a problem hiding this comment.
These were added to keep parity with what we have today, though I need to move pageKey in another PR as it is superfluous. savedAt was added because that is what a page would contain.
| @@ -198,7 +207,7 @@ export abstract class ApplicationBase extends React.Component<Props> { | |||
| } | |||
|
|
|||
| abstract buildStore( | |||
| initialState: RootState, | |||
| initialState: {pages: AllPages, [key: string]: JSONValue}, | |||
There was a problem hiding this comment.
When building a store, superglue takes the injected visit response, transforms it to a page and passes it to buildstore. At that point the superglue key hasn't been built, so we swap root state for something a bit more flex
| if ( | ||
| !( | ||
| action instanceof Object && | ||
| 'type' in action && | ||
| typeof action.type === 'string' | ||
| ) | ||
| ) { | ||
| return nextAction | ||
| } |
There was a problem hiding this comment.
Typescript detected non standard action types, so we narrow the types here. If the shape doesn't match, just return the action
|
|
||
| export type DefermentThunk = ThunkAction< | ||
| Promise<void[]>, | ||
| RootState, | ||
| never, | ||
| undefined, |
There was a problem hiding this comment.
Also fixed a typescript err
| const isSearchable = /^[\da-zA-Z\-_=.]+$/ | ||
| import { JSONMappable, JSONValue } from '../types' | ||
|
|
||
| const canLookAhead = /^[\da-zA-Z\-_]+=[\da-zA-Z\-_]+$/ |
There was a problem hiding this comment.
This changed the regex to testing for any combination of abcABC123-= to
abcABC123-=abcABC123-_. In otherwords, it tests for things like foobar=some123id
| throw new KeyPathError( | ||
| `Expected to find an Array when using the key: ${key}` | ||
| ) |
There was a problem hiding this comment.
You may noticed that I've now hoisted the error throwing from getKey to further up the stack. This made it easier to type atKey and getKey.
| @@ -106,7 +110,7 @@ export class HandlerBuilder { | |||
| linkOrForm: HTMLAnchorElement | HTMLFormElement, | |||
| url: string, | |||
| opts: VisitProps | RemoteProps | |||
| ): Promise<Meta> { | |||
| ): Promise<Meta> | undefined { | |||
There was a problem hiding this comment.
Undefined is possible. Thanks typescript
| // This needs to be done better. This is saying to | ||
| // remove the content-type header from UJS form | ||
| // submissions. | ||
| const fromUJSForm = headers['content-type'] === null |
There was a problem hiding this comment.
Found a better way to todo this. If the body is FormData, we just remove the content-type header. This also changes the tests down below where you'll see removal of the header
Keep parity with existing header usage.
This enables
strictintsconfig.json, and we fix those up!