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

TypeScript: Generators and Templates tracking issue #523

Closed
18 of 42 tasks
thedavidprice opened this issue May 8, 2020 · 11 comments · Fixed by #2279
Closed
18 of 42 tasks

TypeScript: Generators and Templates tracking issue #523

thedavidprice opened this issue May 8, 2020 · 11 comments · Fixed by #2279

Comments

@thedavidprice
Copy link
Contributor

thedavidprice commented May 8, 2020

This is a sub-tracking issue of #234

This is a component of Redwood's initiative to add first-class TypeScript support. 🎉

Generators are a part of the CLI package and located in this directory:
https://github.com/redwoodjs/redwood/tree/master/packages/cli/src/commands/generate

The CLI package is built with Yargs. Read more here.

Goal

The intention is to provide a path for a collaborative, incremental transition to having 100% TypeScript generators. This means the scope of a PR can be an individual generator, which can be merged into the master branch on its own when it's ready.

  • source templates will be .ts
  • generators should be aware of both .ts and .js; this applies to 1) refusing to run if files already exist and 2) overwriting existing files using --force
    • In the case where existing files are a different language target (e.g. in the case of a hybrid-project), the generator should refuse to run regardless of using the --force option; a clear explanation message should be given
  • templates should be transpiled and formatted when the target language is JS (suggested to use ts.transpile and prettier, but this needs evaluation)
    • need to determine appropriate ts.ScriptTarget; "ESNext" seems like a candidate
    • ⚠️transpilation formatting goal == match existing JS templates so that no updates are required for Redwood Tutorial, either code snippets or images (TBD as needed)
  • generators should have CLI options for targeting --javascript or --typescript
    • generators should not require a language target option; instead, the default target will be the project "language default" or a config override in redwood.toml if given.
    • Example: if tsconfig.json exists and there is no redwood.toml override, the generator will automatically set --typescript to true. Therefore, running yarn rw g [option] will target typescript.
    • TODO: create a new Internal package helper for easy access to this config value
  • generators need to pass CI tests and the generated ...test.ts|js files need to pass yarn rw test and yarn rw lint

Steps to Convert a Specific Generator

  1. Rewrite each generator template, including template test file, as TS
    • TS files will replace JS files
  2. Convert corresponding __tests__ directory files and fixtures to TS
  3. Add a JS and TS target CLI command option, e.g. [--javascript|--typescript] to each generator CLI command
    • commands should also be aliased [-js|-ts] respectively
    • set the behavior to automatically use the project "language default" (see above) OR a given --javascript or --typescript option
  4. When language target=javascript, transpile and format TS templates to JS
  • do generated ..test.ts/js files pass yarn rw lint and yarn rw test?

Checklist: Templates and Generators

  • Destroy (New command in progress via Add destroy command #487)
    • confirm support for both ts and js filetypes, as well as hybrid Apps
    • add support as/if needed
  • Helpers (PR CLI: Convert helpers and src/lib to ts #557 stalled out)
    • convert __tests__/helpers.test.js to TS
    • convert helpers.js to TS
  • src/lib Files
    note: these files are outside generators/, but the functionality that relates to generators should be moved to src/commands/generators
    • src/lib/colors.js
    • src/lib/index.js
    • src/lib/test.js
  • Component Convert component generator to TS #632
    • convert templates/ files to TS
    • convert __tests__/ fixtures and test.js to TS
    • generator support for JS transpilation; enable --javascript as default
  • Function
    • convert templates/ file to TS
    • convert __tests__/ fixtures and test.js to TS
    • generator support for JS transpilation; enable --javascript as default
  • Layout Convert layout generator to TypeScript #685
    • convert templates/ files to TS
    • convert __tests__/ fixtures and test.js to TS
    • generator support for JS transpilation; enable --javascript as default
  • Page
    ⚠️ requires updates to Router to support TS
    • convert templates/ files to TS
    • convert __tests__/ fixtures and test.js to TS
    • generator support for JS transpilation; enable --javascript as default
  • SDL 👉PR enable typescript services and sdl #515
    • convert templates/ file to TS
    • convert __tests__/ fixtures and test.js to TS
    • generator support for JS transpilation; enable --javascript as default
  • Service 👉PR enable typescript services and sdl #515
    • convert templates/ files to TS
    • convert __tests__/ fixtures and test.js to TS
    • generator support for JS transpilation; enable --javascript as default
  • Cell
    ⚠️ requires additional support to replicate functionality in TS. Details in this comment.
    • convert templates/ files to TS
    • convert __tests__/ fixtures and test.js to TS
    • generator support for JS transpilation; enable --javascript as default
  • Scaffold
    ⚠️ requires SDL, Service, and Cells TS conversion
    ⚠️ requires updates to Router to support TS
    • convert templates/ files to TS: components, layouts, and pages
    • convert __tests__/ fixtures and test.js to TS
    • generator support for JS transpilation; enable --javascript as default
@jmreidy
Copy link

jmreidy commented May 16, 2020

I've picked up SDL and Service, will post in the forum for volunteers for the rest!

@MarkPollmann
Copy link
Contributor

I would start with Helpers 😎

@jmreidy
Copy link

jmreidy commented May 18, 2020

@MarkPollmann FYI I've extracted the functionality to transform generated TS into prettier-ified JS, you can take a look at my PR or just branch off it.

@kimadeline
Copy link
Contributor

Hello hello, I'd like to pick up the Component work next week. It would be what's under packages/cli/src/commands/generate/component, right?

wait, it appears twice in the checklist 🤔

@cannikin
Copy link
Member

@kimadeline That would be amazing, thank you! None of the core folks are TS experts so any help is greatly appreciated.

Looks like @thedavidprice already updated the list to remove that second reference—we do only have one component generator, honest!

@thedavidprice
Copy link
Contributor Author

Hi @kimadeline! Grateful in advance for the help --> yes, please 🚀

For examples of implementation in progress, do check out the work on #515 and #557 (I'm behind on reviews but will make it a priority this weekend.)

It would be what's under packages/cli/src/commands/generate/component/, right?

^^ yes, indeed

wait, it appears twice in the checklist 🤔

^^ 'cause we are super thorough around here... also double pleasure of checking off lists when done!

(great catch, thank you --> edited 😉)

@thedavidprice
Copy link
Contributor Author

@peterp @kimadeline
Question about specific generator behavior from here:

generator refuses to run if files exist in the same language

^^ original idea was to have the generator refuse if either language exists. But I proposed that without any discussion. I have no strong opinion either way other than to defer to what we think user will expect and either warn or refuse if generator behavior might be unanticipated. And to have all the generators behave the same.

Thoughts/recommendations? 1) refuse on either language 2) refuse on same language 3) middle path (e.g. warn)

@kimadeline
Copy link
Contributor

I have no strong opinion either, mostly from lack of exposure to hybrid projects 🙈 I agree with your other points though, going with what users would expect and unifying generator behaviour.

Now, what do you think users would expect? 🤔

@papaponmx
Copy link
Contributor

Hey, guys. I'd like to help with this adding TS support, is Scaffold is still available?
@peterp @thedavidprice

@thedavidprice
Copy link
Contributor Author

HI @papaponmx! Thanks for jumping in here and offering to help, which would be 🚀

However, the Scaffold generator is blocked by Cells and potentially the Router. Before being able to convert the generators, Cells will have to be completed.

Looping in @peterp

Copy+paste from the linked thread in the OP:

Cells

  • Cells are automatically wrapped as a React Component via a custom webpack loader
  • This means that there is a mismatch between the static type of a Cell file module and the type
    you end up using when you import it.
  • TypeScript has no way of knowing about this change, so when you try to use the cell as a React component, the compiler will complain

Solution?

  • With code generation, we could generate a wrapper for each cell
  • This generated file could be named in such a way that the TypeScript compiler would pick it up (MyCell/index.ts). Just like with every "generated code" based solution, things like "go to definition" would take users to the generated code. Which is probably not what they want.

This is a bit outdated. @peterp do you have direction about the next steps for Cell's TS Support? Also, am I correct in understanding the necessary Router TS support is completed?

@Tobbe
Copy link
Member

Tobbe commented Jan 17, 2021

The Router TS support is in progress here: #1651

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants