Static typing for cells#2208
Conversation
This PR is the first step towards implementing redwoodjs#2205. It allows users to declare the prop types their cells expect. It also renames `withCell` to `createCell`, for the reasons I outlined in redwoodjs#2205 (comment). `createCell` feels like a more parsimonious name to me but I can back that change out if others disagree. It appears that the babel transform for wrapping cells already checks for a default export and bails out if one exists, so this shouldn't require any changes there. The major remaining task is updating the cell generator to add a `export default createCell` line at the end of each cell in Typescript. I'd appreciate more input on whether this is a change we're fine making on both the TS and JS sides, or whether we'll need two templates. Do we have multiple templates anywhere else?
thedavidprice
left a comment
There was a problem hiding this comment.
Hi all! @mojombo has asked us to HODL merging this PR until he's able to dig in later this week / early next. Since he's out this week, he and I decided to not include anything with potential API changes in v0.29 release.
I've added it to the following spring and created a milestone "hold-merge".
Sound good?
|
Update: Core Team had a great discussion about this earlier today. @peterp will be following up with the next steps as we'd like to get this into v0.30 (next release). We'll loop in Tom one more time once we finalize a the discussion topics from earlier today. Thanks again! |
|
Awesome, will wait to hear from @peterp ! |
|
I've updated this so that it's no longer a breaking change - we're not 100% confident that people aren't using the One of the concerns that we also have about this approach is that if a user declares I think this may or may-not be a concern since if the user doesn't export |
Co-authored-by: Daniel Choudhury <[email protected]>
|
Wohoo! Excited this is in. :) |
…erve-web * 'main' of github.com:redwoodjs/redwood: (40 commits) Support generating typescript cells and pages | Autodetect ts project (redwoodjs#2279) create-redwood-app messages: moved app start commands to end (redwoodjs#2278) Add import type to configuration files (redwoodjs#2214) Bump @reach/skip-nav from 0.13.2 to 0.15.0 (redwoodjs#2237) Adding Setup Deploy Render Command (redwoodjs#2099) Disable role linting in Routes (redwoodjs#2318) v0.30.1 (redwoodjs#2322) teardown should attempt to delete dbName table (redwoodjs#2083) Restore @storybook/addon-a11y (redwoodjs#2309) fix(auth): Implement automatic token refresh on supported providers (redwoodjs#2277) fix(cli): move api-server dep from api to cli (redwoodjs#2307) Static typing for cells (redwoodjs#2208) Recommended Babel package upgrades (dependabot) (redwoodjs#2255) v0.30.0 (redwoodjs#2301) upgrade Prisma v2.21.0 (redwoodjs#2273) Further improvements to CONTRIBUTING.md (redwoodjs#2261) Adds better messages for rwt link | Watcher does not exist on build failure | Only remove node_modules after a succesful framework build (redwoodjs#2269) Update named param types in router readme (redwoodjs#2262) Bump core-js from 3.6.5 to 3.10.1 (redwoodjs#2243) Fix: webpack optimizations for JS (redwoodjs#2235) ...
This PR is the first step towards implementing #2205. It allows users to declare the prop types their cells expect.
It also renames
withCelltocreateCell, for the reasons I outlined in #2205 (comment).createCellfeels like a more parsimonious name to me but happy to back this change out if others disagree.It appears that the babel transform for wrapping cells already checks for a default export and bails out if one exists, so this shouldn't require any changes there.
The major remaining task is updating the cell generator to add a
export default createCellline at the end of each cell in Typescript. I'd appreciate more input on whether this is a change we're fine making on both the TS and JS sides, or whether we'll need two templates. Do we have multiple templates anywhere else?