Add TypeScript support to template-registry #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As a first example, this change creates typescript/alter_headers.ts. Then, I ran: `npm run transpile && npm run format`. tsc overwrote javascript/alter_headers.js, and prettier formatted it to match the repo's existing style conventions, resulting in no actual diff to alter_headers.js -- but the code is now generated from TypeScript!
Noteworthy changes: * Response.Body.json() has a return value of type Any; we should explicitly stringify it before possibly passing it as the first argument to the Response constructor. * The Response constructor expects a string as its first argument. The runtime does attempt to stringify arguments passed to it, but we should explicitly join the list of strings in `results` before passing it as an argument to the constructor.
ispivey
commented
May 9, 2020
| @@ -1,7 +1,8 @@ | |||
| { | |||
| "arrowParens": "avoid", | |||
There was a problem hiding this comment.
I added this to conform with the existing snippet conventions in this repo. The vast majority don’t use parens around arguments to arrow functions, so this was least disruptive.
| "singleQuote": true, | ||
| "semi": false, | ||
| "trailingComma": "all", | ||
| "tabWidth": 2, | ||
| "printWidth": 80 | ||
| "tabWidth": 2 |
There was a problem hiding this comment.
This was part of the motivation for including prettier. By default, tsc transpilation introduces 4-space indentation, which would have been disruptive and made it harder to identify changes to the JS snippets.
|
|
|
|
Also, updates code to reflect that you can't pass a Response as the `body` parameter of the Response constructor and expect the resulting new response to inherit properties of the original Response.
This change depends on updating workers-types to define the iterable properties of FormData, like `entries()`.
|
Going to re-open as a PR against upstream. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The goal of this change is to eventually allow us to write snippets in TypeScript, transpile them to JavaScript, and surface both versions in the Template Gallery. By treating the TypeScript snippets as the source-of-truth, we ensure they stay in sync.
This also gives us a good way to catch potential bugs in the JavaScript versions of the snippets!
In the short term, this specific change should effectively be a no-op for the Template Gallery — the API only surfaces the JS files. If folks think this is a good idea, we can do further work to expose the TypeScript snippets in the docs.
Eventually, we should consider removing the transpiled JavaScript snippets from source control entirely. For now, it seems more helpful to keep them in source control so reviewers can see the changes that will result from replacing the JavaScript snippets with TypeScript snippets and transpiling back to JS.
Please consider reviewing commit-by-commit, as commits are small and related in scope.
This change does several things:
npmscript command to runprettier, which is in line with the repo standards, and uses the existing.prettierrc.prettierand commits the small number of resulting changestemplates/typescriptfolder, as well as atranspilecommand that usestscto transpile any TypeScript files in said folder into thetemplates/javascriptfolder.After translating each JS snippet into TS, I ran
npm run transpile && npm run format. If a commit only contains a new TypeScript snippet, that means the transpiled JavaScript version was identical to the JavaScript snippet already in source control.This change does not seek to meaningfully change or improve the snippets themselves, other than to fix bugs or typos in comments.