Refactor/ts scripts #7178

nikkhielseath · 2022-07-25T18:03:21Z

Description

Migrate generate-header-ids script to TypeScript.

Related Issue

Epic: TypeScript rollout #6392

Define a config object to pass in optional parameters to traverse headers

Throw error if no valid path is provided

nikkhielseath · 2022-07-25T18:04:20Z

src/scripts/generate-heading-ids.ts

@@ -13,16 +13,27 @@ const GitHubSlugger = require("github-slugger")

 // TODO we should auto-run this script when new markdown files are added to the project

-const toc = {}
+interface IGitHubSlugger {
+  slug: (headingText: string) => string


refactor: I should install @types/github-slugger

@pettinarip

Are Definitely Typed types detected with a CommonJS require syntax?
I can use typeof gitHubSlugger to specify its type, but, can't use BananaSlug which is the name of its type class definition from the @types/github-slugger package

Or an alternate solution: declare the script as ES6 Module and use the import syntax.

Yes, lets try that. Lets use it as an es6 module where you can do these imports using import.

Reading a bit, I think you will need to override the tsconfig for ts-node to use commonjs module, like this:

// in tsconfig.json add "ts-node": { "compilerOptions": { "module": "commonjs" } }

@pettinarip

For now, I am using the .mts extension but, if you want then, I think we can use moduleTypes to define the module type for this script explicitly.

Also, since module type is inferred from the nearest package.json, we can add a package.json in the scripts folder and specify the type as module for all the scripts.

But, this option would require converting the other scripts to use the ESM import and will go out of the scope of this PR.

Yes, I guess I like to have everything in .ts instead of mixing them with different extensions to make it simpler to other developers. But I don't have a strong opinion about that, I think we are fine with using it as .mts.

src/scripts/generate-heading-ids.ts

nikkhielseath · 2022-07-25T18:07:42Z

tsconfig.json

+    "typeRoots": ["./node_modules/@types"],
+    "types": ["node"]


todo: remove this

src/scripts/generate-heading-ids.ts

nikkhielseath · 2022-07-25T18:11:38Z

src/scripts/generate-heading-ids.ts

  // Sluggers should be per file
-  const slugger = new GitHubSlugger()
+  const slugger: IGitHubSlugger = new GitHubSlugger()


todo: use correct type for GitHub slugger

gatsby-cloud · 2022-07-25T18:36:38Z

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: 29m

pettinarip

Good job @SNikhill. Not sure if this was ready for review as there are a couple of TODO comments.

src/scripts/generate-heading-ids.ts

pettinarip

As mentioned in the last comment, lets try to use es6 imports by configuring the tsconfig file.

In addition, lets

install the github-slugger types
rename this file to generateHeadingIds.ts if everything works as expected

nikkhielseath · 2022-08-05T19:14:31Z

package.json

@@ -100,7 +102,7 @@
    "crowdin-clean": "rm -rf .crowdin && mkdir .crowdin",
    "crowdin-import": "ts-node src/scripts/crowdin-import.ts",
    "format": "prettier --write \"**/*.{js,jsx,json,md}\"",
-    "generate-heading-ids": "node src/scripts/generate-heading-ids.js",
+    "generate-heading-ids": "ts-node --esm src/scripts/generateHeadingIds.mts",


@pettinarip

I am using esm flag to mark it as ES Module along with mts extension for the file

I had to manually set the syntax to typescript in neovim for this mts file to get the syntax highlighting.

nikkhielseath · 2022-08-05T19:15:18Z

src/scripts/generateHeadingIds.mts

-const addHeaderID = (line, slugger, write = false) => {
+const addHeaderID = (
+  line: string,
+  slugger: BananaSlug,


@pettinarip

Type for GitHub Slugger is now inferred. Class Name: BananaSlug

nikkhielseath · 2022-08-05T19:19:14Z

package.json

@@ -90,6 +91,7 @@
    "prettier": "^2.2.1",
    "pretty-quick": "^3.1.0",
    "react-test-renderer": "^17.0.1",
+    "ts-node": "^10.9.1",


I had to list ts-node as a devDependency.

nikkhielseath · 2022-08-05T19:20:13Z

package.json

@@ -100,7 +102,7 @@
    "crowdin-clean": "rm -rf .crowdin && mkdir .crowdin",
    "crowdin-import": "ts-node src/scripts/crowdin-import.ts",
    "format": "prettier --write \"**/*.{js,jsx,json,md}\"",
-    "generate-heading-ids": "node src/scripts/generate-heading-ids.js",
+    "generate-heading-ids": "ts-node --esm src/scripts/generateHeadingIds.mts",


I had to manually set the syntax to typescript in neovim for this mts file to get the syntax highlighting.

pettinarip

lgtm. Ty @SNikhill

nikkhielseath added 9 commits July 23, 2022 16:01

chore(ethereum#6392): rename to ts file

04c4920

chore(tsconfig): set typeroots

1341083

chore(ethereum#6392): set basic types

737930a

chore: addHeaderId & stripLink types

9c8f4da

chore: types for addHeaderIDs

b25ffdc

refactor(traverseHeaders): new function signature

2d78153

Define a config object to pass in optional parameters to traverse headers

fix(generateHeadings): throw error on no path

5a52113

Throw error if no valid path is provided

chore(addHeaderId): add return type

92dd6b7

refactor: rename toc to headersToChange

6ada1de

nikkhielseath requested review from samajammin, wackerow, corwintines, pettinarip and minimalsm as code owners July 25, 2022 18:03

github-actions bot added dependencies 📦 Changes related to project dependencies review needed 👀 tooling 🔧 Changes related to tooling of the project labels Jul 25, 2022

nikkhielseath commented Jul 25, 2022

View reviewed changes

pettinarip added the typescript-migration label Jul 26, 2022

pettinarip reviewed Jul 26, 2022

View reviewed changes

src/scripts/generate-heading-ids.ts Outdated Show resolved Hide resolved

src/scripts/generate-heading-ids.ts Outdated Show resolved Hide resolved

minimalsm assigned pettinarip Jul 27, 2022

nikkhielseath added 2 commits July 29, 2022 20:40

fix(tsconfig): remove typeroots

9cc1a71

fix(type): use typeof for githubSlugger

893b5ee

nikkhielseath requested a review from jmcook1186 as a code owner July 29, 2022 15:38

pettinarip requested changes Aug 1, 2022

View reviewed changes

nikkhielseath added 2 commits August 6, 2022 00:37

chore(package): add ts-node & github-slugger type

dc64724

refactor(addHeaderId): convert to ESModule

136c4f9

nikkhielseath commented Aug 5, 2022

View reviewed changes

pettinarip approved these changes Aug 8, 2022

View reviewed changes

pettinarip merged commit b5d591a into ethereum:dev Aug 8, 2022

nikkhielseath deleted the refactor/ts-scripts branch August 8, 2022 18:51

minimalsm mentioned this pull request Aug 8, 2022

Deploy v4.11.0 #7342

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor/ts scripts #7178

Refactor/ts scripts #7178

nikkhielseath commented Jul 25, 2022

nikkhielseath Jul 25, 2022

nikkhielseath Jul 29, 2022

nikkhielseath Jul 29, 2022

pettinarip Aug 1, 2022

nikkhielseath Aug 5, 2022

nikkhielseath Aug 5, 2022

pettinarip Aug 8, 2022

nikkhielseath Jul 25, 2022

nikkhielseath Jul 25, 2022

gatsby-cloud bot commented Jul 25, 2022 •

edited

Loading

pettinarip left a comment

pettinarip left a comment

nikkhielseath Aug 5, 2022

nikkhielseath Aug 5, 2022

nikkhielseath Aug 5, 2022

nikkhielseath Aug 5, 2022

nikkhielseath Aug 5, 2022

pettinarip left a comment

Refactor/ts scripts #7178

Refactor/ts scripts #7178

Conversation

nikkhielseath commented Jul 25, 2022

Description

Related Issue

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

gatsby-cloud bot commented Jul 25, 2022 • edited Loading

Gatsby Cloud Build Report

Build Details

pettinarip left a comment

Choose a reason for hiding this comment

pettinarip 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

pettinarip left a comment

Choose a reason for hiding this comment

gatsby-cloud bot commented Jul 25, 2022 •

edited

Loading