Typescript migration #6307

pettinarip · 2022-05-12T13:46:25Z

First iteration on TS migration (#6392).

Description

Core files has been migrated.

Some extra things:

Package scripts were refactored a bit. The prebuild script processes has been moved into onPreBootstrap hook in gatsby-node.
Migrated data/translations.json into utils/languages.ts.
Removed unused LegacyPageHome.js page.
Decoupled a but the logic that lives inside the translations.ts file. I moved them into the languages.ts.

More reference: https://www.gatsbyjs.com/docs/how-to/custom-configuration/typescript/#migrating-to-typescript

TODO

migrate and see how to test the docsearchConfigScript.js file
fix GC build issue

gatsby-cloud · 2022-05-12T13:49:08Z

Gatsby Cloud Build Report

ethereum-org-website-dev

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

Build Details

View the build logs here.

🕐 Build time: 24m

Performance

Lighthouse report

Metric	Score
Performance	🔶 24
Accessibility	💚 100
Best Practices	💚 100
SEO	💚 92

🔗 View full report

tsconfig.json

gatsby-config.ts

.github/workflows/docsearch-crawl.yml

wackerow

This looks awesome ❤️
Site is building properly locally, and not getting any issues with any pages breaking...

...Dare I say we're on the verge of Deploy v4.0.0??

nhsz

Awesome, great job! 🎉 Just left a couple of suggestions

nhsz · 2022-05-18T15:54:08Z

tsconfig.json

+    "skipLibCheck": true,
+    "resolveJsonModule": true
+  },
+  "include": ["./src"]


we should add the "exclude": ["node_modules"] prop here for faster compilation

💭 I wonder what you lose from doing that. Do you know if that is not equivalent of doing "skipLibCheck": true (which we are already doing). https://www.typescriptlang.org/tsconfig#skipLibCheck

Not sure if it has the same effect

nhsz · 2022-05-18T15:55:41Z

types.ts

@@ -0,0 +1,21 @@
+import { Lang } from "./src/utils/languages"
+import { IMessages } from "./src/utils/flattenMessages"


maybe we can move the IMessages interface (and other defined interfaces) to an interfaces.ts file, like we do with types.ts. I'd also suggest to remove the I from the interface name (and use just Messages), to standardize the naming

Ok, I will standardize these names without the I but not sure about putting them into a single file.

Let me know what do you think about the following. Maybe we can have:

interfaces.ts file -> for global or cross-app interfaces

each [module].ts -> interfaces related to that module

Basically to avoid having a huge interfaces.ts file and to quickly find the interfaces related to the code.

I guess this is more the convention we want to follow as a team.

@pettinarip agree with that convention

package.json

samajammin · 2022-05-18T17:01:22Z

One small thought (that isn't a blocker on this PR): should /utils/translations.ts & /utils/languages.ts be separate files? Or should we just merge them?

samajammin · 2022-05-18T17:01:48Z

One note, we'll have to account for at least one recent PR: #6366

samajammin · 2022-05-18T17:04:21Z

src/utils/getMessages.ts

@@ -0,0 +1,27 @@
+import fs from "fs"


Organization nitpick question - do getMessages & flattenMessages both need their own files? They both have to do with i18n support (specifically the gatsby-plugin-intl plugin).

Nice idea. I tried to avoid doing a lot of refactors just to keep this PR as much as scoped as possible but your suggestion makes total sense.

I will take note of this and implement that in the next PR 👍🏼

samajammin · 2022-05-18T17:06:42Z

src/scripts/__tests__/merge-translations.js

@@ -1,4 +1,4 @@
-import { mergeObjects } from "../merge-translations"
+import mergeObjects from "../../utils/mergeObjects"


I assume we'll eventually want to migrate these test files as well?

(I'm ok if it's not this PR).

Yes! will migrate them in next PRs 💪🏼

nloureiro · 2022-05-18T17:33:55Z

I pulled the PR, and local got this error; not sure if it's something on my end, but I got this.

corwintines · 2022-05-18T20:56:12Z

I pulled the PR, and local got this error; not sure if it's something on my end, but I got this.

This has to do with the hot reload module. Usually a refresh fixes this. Happens sometimes when changing code and the cache coming out of sync I believe.

corwintines

Nice @pettinarip! I would have approved, but there's a conflict. Let me know if you need a hand!

pettinarip · 2022-05-19T00:53:18Z

One small thought (that isn't a blocker on this PR): should /utils/translations.ts & /utils/languages.ts be separate files? Or should we just merge them?

I separated them on purpose because:

translations.ts depends on the existence of src/intl/en.json. The latter is generated on build time by mergeTranslations that is called by gatsby-node, which needs some functions from languages.ts. If we merge them, we will have a conflict.
and for organization matters, to have smaller files with related things.

One note, we'll have to account for at least one recent PR: #6366
Thanks, yes, I'll resolve the conflicts soon and include that PR content.

pettinarip · 2022-05-19T00:53:53Z

Nice @pettinarip! I would have approved, but there's a conflict. Let me know if you need a hand!

Thanks for the review @corwintines! Yes, tomorrow I'll resolve the conflicts and merge it.

samajammin

Made some comments but no blockers in my opinion. Great work!

Only (minor) concern I have is that while yarn start works fine, netlify dev no longer works on my local env:
https://discord.com/channels/714888181740339261/974453286134485072/976522784891084870

But seems like that is just an issue for me? e.g. @wackerow said everything works fine for him.

pettinarip · 2022-05-19T18:34:24Z

....aand we have a successful build here https://www.gatsbyjs.com/dashboard/5e02f963-3fe5-4e37-a1e7-ac05f7753a86/sites/dc7c4340-a8e9-44ed-b7be-883c70d2f02d/builds/daba9e15-732d-4d24-8de1-90ac7e00253a/details

pettinarip added 11 commits April 14, 2022 11:17

initial core files migration to ts

e09805c

add graphql codegen

447acce

ignore from prettier the generated graphql types file

037101d

Merge branch 'dev' into ts

53dcf69

gatsby ssr to ts

598aaf9

rerun graphql types

de72fc0

gatsby browser to ts

d275f54

remove auto generated queries types

399c94a

rename translations file to languages

180eee8

cleanup

c0056f4

Merge branch 'dev' into ts

fbb16b9

pettinarip requested review from samajammin, wackerow, corwintines and minimalsm as code owners May 12, 2022 13:46

github-actions bot added content 🖋️ This involves copy additions or edits dependencies 📦 Changes related to project dependencies review needed 👀 tooling 🔧 Changes related to tooling of the project labels May 12, 2022

pettinarip commented May 12, 2022

View reviewed changes

tsconfig.json Show resolved Hide resolved

pettinarip commented May 12, 2022

View reviewed changes

gatsby-config.ts Show resolved Hide resolved

move languages functions from translations.ts to languages.ts

edf6973

github-actions bot added the translation 🌍 This is related to our Translation Program label May 12, 2022

pettinarip removed content 🖋️ This involves copy additions or edits translation 🌍 This is related to our Translation Program labels May 12, 2022

fix bad imports

0e84cc8

github-actions bot added content 🖋️ This involves copy additions or edits translation 🌍 This is related to our Translation Program labels May 12, 2022

pettinarip marked this pull request as draft May 13, 2022 14:10

pettinarip force-pushed the ts branch from f90d280 to 8663543 Compare May 13, 2022 20:16

exec docsearch script with ts-node

0bc9bcf

pettinarip force-pushed the ts branch from 8663543 to 0bc9bcf Compare May 13, 2022 20:52

pettinarip commented May 13, 2022

View reviewed changes

.github/workflows/docsearch-crawl.yml Show resolved Hide resolved

pettinarip marked this pull request as ready for review May 13, 2022 21:10

wackerow approved these changes May 17, 2022

View reviewed changes

nhsz approved these changes May 18, 2022

View reviewed changes

samajammin reviewed May 18, 2022

View reviewed changes

package.json Show resolved Hide resolved

samajammin reviewed May 18, 2022

View reviewed changes

pettinarip mentioned this pull request May 18, 2022

Epic: TypeScript rollout #6392

Closed

corwintines requested changes May 18, 2022

View reviewed changes

samajammin approved these changes May 19, 2022

View reviewed changes

pettinarip added 3 commits May 19, 2022 14:07

refactor our types and interfaces files to be a bit more organized

b81d89f

Merge branch 'dev' into ts

2c08f7e

add da lang to our languages file due to merge conflicts

4a28461

pettinarip force-pushed the ts branch from 6c40f7e to 4a28461 Compare May 19, 2022 17:31

pettinarip merged commit ea6d453 into dev May 19, 2022

pettinarip deleted the ts branch May 19, 2022 18:35

This comment was marked as spam.

Sign in to view

pettinarip mentioned this pull request May 25, 2022

Deploy v4.0.0 #6469

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Typescript migration #6307

Typescript migration #6307

pettinarip commented May 12, 2022 •

edited by samajammin

Loading

gatsby-cloud bot commented May 12, 2022 •

edited

Loading

wackerow left a comment

nhsz left a comment

nhsz May 18, 2022

pettinarip May 19, 2022

nhsz May 23, 2022

nhsz May 18, 2022 •

edited

Loading

pettinarip May 19, 2022

nhsz May 23, 2022

samajammin commented May 18, 2022

samajammin commented May 18, 2022

samajammin May 18, 2022

pettinarip May 19, 2022

This comment was marked as spam.

samajammin May 18, 2022

samajammin May 18, 2022

pettinarip May 19, 2022

nloureiro commented May 18, 2022

corwintines commented May 18, 2022

corwintines left a comment

pettinarip commented May 19, 2022

pettinarip commented May 19, 2022

samajammin left a comment

pettinarip commented May 19, 2022

This comment was marked as spam.

		@@ -0,0 +1,21 @@
		import { Lang } from "./src/utils/languages"
		import { IMessages } from "./src/utils/flattenMessages"

		@@ -1,4 +1,4 @@
		import { mergeObjects } from "../merge-translations"
		import mergeObjects from "../../utils/mergeObjects"

Typescript migration #6307

Typescript migration #6307

Conversation

pettinarip commented May 12, 2022 • edited by samajammin Loading

Description

TODO

gatsby-cloud bot commented May 12, 2022 • edited Loading

Gatsby Cloud Build Report

Build Details

Performance

Lighthouse report

wackerow left a comment

Choose a reason for hiding this comment

nhsz 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

nhsz May 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samajammin commented May 18, 2022

samajammin commented May 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as spam.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nloureiro commented May 18, 2022

corwintines commented May 18, 2022

corwintines left a comment

Choose a reason for hiding this comment

pettinarip commented May 19, 2022

pettinarip commented May 19, 2022

samajammin left a comment

Choose a reason for hiding this comment

pettinarip commented May 19, 2022

This comment was marked as spam.

pettinarip commented May 12, 2022 •

edited by samajammin

Loading

gatsby-cloud bot commented May 12, 2022 •

edited

Loading

nhsz May 18, 2022 •

edited

Loading