TypeScript versions of the homepage starters #95

aghreed · 2022-03-08T19:09:25Z

Purpose

This PR sets up the ability to handle publishing TS and JS "source" versions of the starters. It also converts all the components over to .tsx as the first step to add type safety to the front-end of the starter.

Testing

These are currently deployed to Gatsby Cloud:
Contentful: https://gatsbycontentfulhomepagets.gatsbyjs.io/
DatoCMS: https://gatsbydatocmshomepagets.gatsbyjs.io/
Drupal: https://gatsbydrupalhomepagets.gatsbyjs.io/
WP: https://gatsbywordpresshomepagets.gatsbyjs.io/

What's included?

Updates to the publish.js script to support transpiling the TS source to "source" JS as well as publishing the four new TS flavors of the starters
Conversion of all front end components and pages to TS
Updates to documentation to account for differences in the READMEs for TS flavors of the starters

gatsby-cloud · 2022-03-08T19:11:30Z

Gatsby Cloud Build Report

homepage-starters-dev

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 29s

Performance

Lighthouse report

Metric	Score
Performance	💚 99
Accessibility	💚 97
Best Practices	💚 100
SEO	💚 92

🔗 View full report

src/components/caret.tsx

… transpilation on necessary repos and calling prettier before writing to file

…e .ts extension - update webpack resolve alias for ui.js

…te webpack alias in conjunction

scripts/data.js

wardpeet

I'm not sure why you're not doing it all in one PR but, i've added some comments

wardpeet · 2022-03-13T15:52:59Z

gatsby-node.js

          __dirname,
          "plugins",
          plugin,
          "src",
-          "colors.css.ts"
+          "colors.css"


Any reason why you don't add ts extension here? Or you remove them anywhere or you add them everywhere but I think the goal here is to remove them everywhere.

Yep, we don't want them around anywhere. For VSCode users (myself included), you need to omit the .ts extension in the import statement in order for VSCode's in-IDE type checking and auto-complete functionality to work properly. And when I did that across the board in this PR, the webpack alias also needed updating.

If I were to add the .ts back to this line, I consistently get this build error:

It sounds like Vanilla Extract still handles these correctly without the extension, right? It's unclear from the documentation whether that's a requirement versus just a convention, but 👍 if this all works

Lennart's example repo omits .ts from import statements as well

scripts/data.js

wardpeet · 2022-03-13T16:02:17Z

src/components/about-leadership.tsx

+  jobTitle?: string
+}
+
+function AboutProfile(props: AboutProfileProps) {


Don't you get type warnings? In React land with typescript, you should do

Suggested change

function AboutProfile(props: AboutProfileProps) {

const AboutProfile: React.FC<AboutProfileProps> = function AboutProfile(props: AboutProfileProps) {

Nope, no type warnings!

I can certainly update to type all the components with React.FC, however I was trying to minimize the changes to the source code in my effort to convert over to TypeScript, and additionally, I tend to agree with the opinions articulated in this article: https://fettblog.eu/typescript-react-why-i-dont-use-react-fc/

I'm not sure what the correct answer to this would be, but I'd say we should follow any "blessed" practices that might exist from React and ensure that our Gatsby style is consistent across the board. @wardpeet do we have much of a precedent set with other starters or TS docs, etc.?

jxnblk

Looking great so far! I'll take another pass at this in the morning, but wanted to comment mostly around the UI components and leave some notes about some of the things we chatted about earlier for consideration/discussion

scripts/data.js

src/components/about-hero.tsx

jxnblk · 2022-03-15T20:53:04Z

src/components/about-leadership.tsx

+  jobTitle?: string
+}
+
+function AboutProfile(props: AboutProfileProps) {


I'm not sure what the correct answer to this would be, but I'd say we should follow any "blessed" practices that might exist from React and ensure that our Gatsby style is consistent across the board. @wardpeet do we have much of a precedent set with other starters or TS docs, etc.?

jxnblk · 2022-03-15T20:56:02Z

src/components/caret.tsx

+
+export default function CaretDown({
+  direction = "down",
+  size = 9,


I don't think we use the size prop at all; I'd be keen to remove it to simplify the API a little

src/components/ui.css.ts

Convert Pages to TS

scripts/data.js

…working dependency versions are installed

…script

scripts/create-readmes.mjs

jxnblk

Think this is looking very close. Left a few comments and questions but nothing major from what I can see

scripts/publish.js

jxnblk · 2022-03-22T15:18:13Z

scripts/publish.js

+        const dest = path.join(
+          dir.dist,
+          name,
+          srcFilename.replace(extension, ".js")


This will likely work, but I think the better way to replace the extension is by using path.basename (i.e. in the odd case that you had a file named foo.ts.bar.ts)

jxnblk · 2022-03-24T20:33:02Z

docs/readme-template.md

@@ -125,8 +128,8 @@ For this example, we'll create a new "Banner" component.

 1. Next, create the Banner component:

-   ```jsx
-   // src/components/banner.js
+   ```jsx fileExt


Haven't checked yet, but I'm guessing the jsx syntax highlighting on GitHub works for tsx syntax, but if it doesn't 100%, I would guess it'd work the other way around. Then again, maybe it doesn't matter because there are significant changes to the code examples' contents

Yeah I thought about that, but didn't think it was worth the trouble because the syntax highlighting won't be different. If we did add in an interface for the props, that wouldn't be highlighted the same way with jsx and tsx but I figure people will be able to figure out adding:

interface BannerProps { heading: string; text: string; }

without us holding their hands on this one.

scripts/create-readmes.mjs

jxnblk · 2022-03-24T20:51:27Z

src/components/ui.css.ts

 export const radii = styleVariants(theme.radii, (borderRadius) => ({
  overflow: "hidden",
  borderRadius,
 }))
-export const order = styleVariants([0, 1, 2, 3], (order) => ({
+export const order = styleVariants({ 0: 0, 1: 1, 2: 2, 3: 3 }, (order) => ({


Is this not typed to accept array objects in vanilla extract?

Yeah, styleVariants is typed to accept a Record as it's first argument

jxnblk · 2022-03-24T20:53:27Z

src/components/ui.css.ts

  {
    ...theme.space,
    auto: "auto",
  },
  (marginTop) => ({ marginTop })
 )
-margin.bottom = styleVariants(
+
+export const marginBottom = styleVariants(


Not a problem, but this just feels less elegant than grouping into an object, but keeps things more consistent

Yeah I changed this one because of the TS intellisense warning:

jxnblk · 2022-03-24T20:55:21Z

src/components/ui.tsx

-        styles.margin.right[-right],
-        styles.margin.top[-top],
-        styles.margin.bottom[-bottom],
+        left && styles.marginLeft[-left],


I thought these props could be 0, but I might be wrong

They could, but there would be no point in passing 0 as the value since you wouldn't be "nudging" at all.

In which case, this would pass a 0 to the className, right?

Ah, no I'm wrong, this goes through .filter(Boolean)

src/pages/404.tsx

…the note about TS & JS sister repos

jxnblk

Looks great! Thanks for getting this all together and getting the readme generation in there as well!

jxnblk · 2022-03-28T20:20:55Z

scripts/create-readmes.mjs

+          )
+        }
+        node.value = node.value.replace("$fileExt", opts.fileExt)
+      }


Looks good! A lot simpler and doesn't mix up the directive syntax

aghreed added 2 commits March 8, 2022 10:11

setup to use tsc

7f36c25

working initial pass on transpilation within publish script

f16f915

aghreed commented Mar 8, 2022

View reviewed changes

src/components/caret.tsx Show resolved Hide resolved

aghreed added 4 commits March 8, 2022 16:42

adjust data.js content and publish script to accommodate only running…

25ffa94

… transpilation on necessary repos and calling prettier before writing to file

convert ui.js to ui.tsx - update all .css.ts imports to explicitly us…

50ab14d

…e .ts extension - update webpack resolve alias for ui.js

convert over all the src/components to ts

091db7d

wip converting pages to tsx

7b32ace

aghreed changed the base branch from main to chore/typescript-all-the-things March 11, 2022 19:15

aghreed changed the title ~~Support TS variations of the starters~~ Update publish script to support TS publishing and convert all src/components to TS Mar 11, 2022

aghreed added 2 commits March 11, 2022 14:35

fix logic in publish script

477c60e

remove .ts file extension from import of vanilla-extract files - upda…

d246222

…te webpack alias in conjunction

aghreed commented Mar 11, 2022

View reviewed changes

scripts/data.js Outdated Show resolved Hide resolved

updates after self-review

111f7af

aghreed marked this pull request as ready for review March 11, 2022 22:02

aghreed requested a review from jxnblk March 11, 2022 22:02

use more types in components

97e0750

wardpeet reviewed Mar 14, 2022

View reviewed changes

aghreed added 3 commits March 14, 2022 14:40

merge in work from components branch

80c7413

convert pages to TS and flesh out sections.tsx functionality

25ca6ce

resolve newline issue during transpilation with code comments

e7eef40

aghreed mentioned this pull request Mar 15, 2022

Convert Pages to TS #97

Merged

update publish script and data file

6add91f

jxnblk reviewed Mar 15, 2022

View reviewed changes

aghreed added 2 commits March 16, 2022 11:16

use union types of string literals rather than enums

222a545

Merge pull request #97 from gatsbyjs/chore/ts-pages-components

513d184

Convert Pages to TS

aghreed changed the title ~~Update publish script to support TS publishing and convert all src/components to TS~~ Update publish script to support TS publishing and convert all src to TS Mar 16, 2022

aghreed added 2 commits March 16, 2022 16:21

revert to old sections component implementation but with types

db1d92a

remove unused type

db9829b

jxnblk reviewed Mar 17, 2022

View reviewed changes

scripts/data.js Outdated Show resolved Hide resolved

aghreed added 3 commits March 17, 2022 17:06

tweak publish script for TS concerns

8f9d74a

add yarn.lock file to list of files copied over in publish to ensure …

2e75135

…working dependency versions are installed

support the TS flavors of the READMEs and handle renaming in publish …

e43d532

…script

aghreed commented Mar 23, 2022

View reviewed changes

scripts/create-readmes.mjs Outdated Show resolved Hide resolved

aghreed added 3 commits March 23, 2022 17:12

merge in main

293707b

adjust publish script to remove about.tsx from ts wordpress starter

efb2de8

add deleted array to checks for when to commit in publish script

1c89da1

aghreed changed the title ~~Update publish script to support TS publishing and convert all src to TS~~ TypeScript versions of the homepage starters Mar 23, 2022

aghreed requested review from jxnblk and wardpeet March 23, 2022 21:46

aghreed changed the base branch from chore/typescript-all-the-things to main March 23, 2022 21:46

jxnblk reviewed Mar 24, 2022

View reviewed changes

aghreed added 3 commits March 28, 2022 09:45

remove strict mode flag - update per PR feedback

103cc7d

update how fileExt in inline code are replaced in READMEs - rephrase …

e8040ea

…the note about TS & JS sister repos

merge in latest from main

04eb709

aghreed requested a review from jxnblk March 28, 2022 19:30

aghreed added 3 commits March 28, 2022 15:34

re-add Fallback component for when blocktype does not match a component

c15a69f

tweaks to publish script

2511ce8

update note on TS and JS versions

dd924ff

jxnblk approved these changes Mar 28, 2022

View reviewed changes

aghreed merged commit dd3ab9a into main Mar 28, 2022

aghreed deleted the aghreed/sc-47188/spike-add-a-ts-variation-to-the-homepage branch March 28, 2022 21:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TypeScript versions of the homepage starters #95

TypeScript versions of the homepage starters #95

aghreed commented Mar 8, 2022 •

edited

Loading

gatsby-cloud bot commented Mar 8, 2022 •

edited

Loading

wardpeet left a comment •

edited

Loading

wardpeet Mar 13, 2022 •

edited

Loading

aghreed Mar 14, 2022

jxnblk Mar 15, 2022

aghreed Mar 15, 2022

wardpeet Mar 13, 2022

aghreed Mar 14, 2022

jxnblk Mar 15, 2022

jxnblk left a comment

jxnblk Mar 15, 2022

jxnblk Mar 15, 2022

jxnblk left a comment

jxnblk Mar 22, 2022

jxnblk Mar 24, 2022

aghreed Mar 28, 2022

jxnblk Mar 24, 2022

aghreed Mar 24, 2022

jxnblk Mar 24, 2022

aghreed Mar 24, 2022

jxnblk Mar 24, 2022

aghreed Mar 28, 2022

jxnblk Mar 28, 2022

jxnblk Mar 28, 2022

jxnblk left a comment

jxnblk Mar 28, 2022

	function AboutProfile(props: AboutProfileProps) {
	const AboutProfile: React.FC<AboutProfileProps> = function AboutProfile(props: AboutProfileProps) {

TypeScript versions of the homepage starters #95

TypeScript versions of the homepage starters #95

Conversation

aghreed commented Mar 8, 2022 • edited Loading

Purpose

Testing

What's included?

gatsby-cloud bot commented Mar 8, 2022 • edited Loading

Gatsby Cloud Build Report

Build Details

Performance

Lighthouse report

wardpeet left a comment • edited Loading

Choose a reason for hiding this comment

wardpeet Mar 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jxnblk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jxnblk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jxnblk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aghreed commented Mar 8, 2022 •

edited

Loading

gatsby-cloud bot commented Mar 8, 2022 •

edited

Loading

wardpeet left a comment •

edited

Loading

wardpeet Mar 13, 2022 •

edited

Loading