Rework typescript guide #1530
Rework typescript guide #1530
Conversation
|
@mtrivera can you resolve the conflicts so the CI build can run? Aside from that I, and hopefully @TheDutchCoder, will review as soon as we have some time (likely later today or tomorrow for me). |
content/guides/typescript.md
Outdated
|
|
||
| ``` bash | ||
| npm install --save-dev typescript ts-loader | ||
| ``` | ||
|
|
||
| Now we'll modify the directory structure & the configuration files: | ||
|
|
||
| __project__git stat |
There was a problem hiding this comment.
Is this a type? I guess it should just be project ? ;)
content/guides/typescript.md
Outdated
| @@ -48,9 +60,13 @@ See [TypeScript's documentation](https://www.typescriptlang.org/docs/handbook/ts | |||
|
|
|||
| __webpack.config.js__ | |||
There was a problem hiding this comment.
Could you make this a diff instead? There's already a config file, so it might be good to show what changed form the "Getting Started" end state of the file.
content/guides/typescript.md
Outdated
| } | ||
| }; | ||
| ``` | ||
|
|
||
| This will direct webpack to _enter_ through `./index.ts`, _load_ all `.ts` and `.tsx` files through the `ts-loader`, and _output_ a `bundle.js` file in our current directory. | ||
|
|
||
| This will direct webpackgit sdt to _enter_ through `./index.ts`, _load_ all `.ts` and `.tsx` files through the `ts-loader`, and _output_ a `bundle.js` file in our current directory. |
There was a problem hiding this comment.
"webpackgit sdt" typo? ;)
content/guides/typescript.md
Outdated
| @@ -48,9 +60,13 @@ See [TypeScript's documentation](https://www.typescriptlang.org/docs/handbook/ts | |||
|
|
|||
| __webpack.config.js__ | |||
|
|
|||
| To learn more about webpack configuration, see the [configuration concepts](/concepts/configuration/). | |||
|
|
|||
| Now let's configure webpack to handle TypeScript: | |||
|
|
|||
| ```js | |||
There was a problem hiding this comment.
it's preferred to use single quotes everywhere for consistency. (the extensions array)
content/guides/typescript.md
Outdated
|
|
||
|
|
||
| ## Source Maps | ||
|
|
||
| To learn more about Source Maps, see the [development guide](/guides/development.md). |
There was a problem hiding this comment.
Can you link directly to that sub-section please? (/guides/development/#using-source-maps)
content/guides/typescript.md
Outdated
|
|
||
|
|
||
| ## Source Maps | ||
|
|
||
| To learn more about Source Maps, see the [development guide](/guides/development.md). | ||
|
|
||
| To enable source maps, we must configure TypeScript to output inline source maps to our compiled JavaScript files. The following line must be added to our `tsconfig.json`: | ||
|
|
||
| ``` json |
There was a problem hiding this comment.
To keep it consistent, please make this a diff of your entire file. It's easier to read for beginners.
There was a problem hiding this comment.
Same for the webpack config changes below this.
content/guides/typescript.md
Outdated
| @@ -124,7 +137,7 @@ For more information see [this blog post](https://blogs.msdn.microsoft.com/types | |||
|
|
|||
| ## Importing Other Assets | |||
|
|
|||
| To use non code assets with TypeScript, we need to defer the type for these imports. This requires a `custom.d.ts` file which signifies custom definitions for TypeScript in our project. Let's set up a declaration for `.svg` files: | |||
| To use non-code assets with TypeScript, we need to defer the type for these imports. This requires a `custom.d.ts` file which signifies custom definitions for TypeScript in our project. Let's set up a declaration for `.svg` files: | |||
|
|
|||
| __custom.d.ts__ | |||
There was a problem hiding this comment.
Can you please use a diff on the project to show where the file goes in the folder structure? Thanks!
content/guides/typescript.md
Outdated
|
|
||
| ## Build Performance | ||
|
|
||
| W> This may degrade build performance. |
There was a problem hiding this comment.
What exactly impacts build performance? Using TypeScript in general?
Maybe be a bit more explicit about what "This" means in this context :)
|
Thanks for the feedback. Yes, they were some typos but I wanted to make sure the guide was on course. Also, I may remove the Build Performance section, since it conflicts with the Production warning in the Build Performance guide. |
skipjack
force-pushed
the
rework-typescript-guide
branch
from
0a92370 to
55fab71
Compare
|
@mtrivera I rebased to resolve the conflicts and, in doing so, also resolved some, but not all, of the points @TheDutchCoder brought up. Please re-read his comments and ping us when you've had a chance to make the updates. Once that's done we can do a final review and get this merged 👍 ! Thanks again for you work! |
Make some minor updates for consistency with the other guides such doing full `diff` blocks and displaying file names directly above examples. Also removed the conclusion as the next guide is not really a logical next step. Things just get kind of random after this guide (something we'll try to clean up at the end of webpack#1258).
There was a problem hiding this comment.
Thanks for your work on this @mtrivera. Made one commit with some minor changes just so we could push this forward. Feel free to submit any other changes in another PR.
Once the build passes, I'll merge and update #1258. @TheDutchCoder if I missed anything, please point it out and we can resolve in a separate PR.
|
Thanks!, I'll open another PR once the build passes. |
Begun typescript rewrite according to other guide.