Skip to content
This repository has been archived by the owner on Aug 26, 2022. It is now read-only.

Docs: TypeScript docs updates #524

Merged
merged 2 commits into from
Jan 24, 2021

Conversation

msutkowski
Copy link
Contributor

There is an additional issue here when working with scaffolded code and the file structure, but I don't know if it is worth addressing with this PR. redwoodjs/redwood#234 (comment) regarding the DirectoryName convention. TS users would just have to:

  1. add an index.ts to src/components/WhateverComponent'
  2. export { default as default } from './WhateverComponent'

As a side note, I didn't see any specific plan for handling this in the tracking issue: redwoodjs/redwood#523

Thanks!

README.md Outdated
@@ -10,6 +10,8 @@ Other documentation is pulled from various READMEs in the main [redwoodjs/redwoo

This codebase is built with https://cameronjs.com and relies on plain HTML pages and Javascript with a couple helpers built in to abstract things like partials and layouts. We use https://stimulusjs.org for the few sprinkles of interactivity throughout the site.

First, make sure that you are running Node 14+. If you're not sure of how to manage your node versions, see [nvm](https://github.com/nvm-sh/nvm).
Copy link
Member

Choose a reason for hiding this comment

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

Maybe mention https://github.com/coreybutler/nvm-windows for windows users. Unfortunately I haven't used it myself, so I can't vouch for it.

Copy link
Contributor Author

@msutkowski msutkowski Jan 17, 2021

Choose a reason for hiding this comment

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

I can vouch that it works as I do use it myself, but I figured someone would've made their way down that rabbit hole.

Just an FYI, the only major difference is that the node version is set globally when using nvm-windows, unlike normal nvm where it's per shell 😓

+"types": ["jest"]
```

Currently, these are added to `node_modules` by `@redwoodjs/core` and the above approach should just work. If this is not the case, you can `npm i -D @types/jest` in the `web` folder and they will resolve.
Copy link
Member

Choose a reason for hiding this comment

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

Should we use yarn instead of npm here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noted and updated!

Copy link
Contributor

Choose a reason for hiding this comment

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

also might be worth saying tsconfig in web or api as we make two and you want to make sure they put it in the right place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@burnsy Thanks for the feedback! This is under the WEB section, but I guess it wouldn't hurt to add additional clarification language?

It might be a good idea to just add in a quick cli generator for tsconfigs that adds to the root and extends in the respective directories for overrides? I'm not sure if that's already a thing in the TS support meta tracker though.

@Tobbe
Copy link
Member

Tobbe commented Jan 17, 2021

@msutkowski Thanks for this! Great input 👍 I just left a couple of comments. @peterp can you have a look as well please?

@msutkowski
Copy link
Contributor Author

@msutkowski Thanks for this! Great input 👍 I just left a couple of comments. @peterp can you have a look as well please?

Just to follow up on this - I went down the 🐰 🕳️ of the TS repo trying to figure out what actually happens with this change... and long story short is that it probably doesn't matter. Any TS user is going to include additional types (as shown with the Jest comment) , but maybe I'm wrong :D . If I am, please let me know, thanks! 🎉

@peterp peterp merged commit f78c047 into redwoodjs:main Jan 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants