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

Configure TypeScript for static-site #877

Merged
merged 4 commits into from
May 28, 2024

Conversation

victorlin
Copy link
Member

@victorlin victorlin commented May 24, 2024

Description of proposed changes

See commit messages

Related issue(s)

Checklist

  • npm run dev doesn't overwrite TSConfig file locally
  • Checks pass
  • Check if changes affect the resource index JSON revision
  • Post-merge: Create issue to enable noImplicitAny (done)

I let `npm run dev` rewrite the file which added some options and
applied some whitespace reformatting. To minimize the amount of changes
in this commit, I restored the original whitespace reformatting then
re-added the Next.js options.
@victorlin victorlin requested a review from a team May 24, 2024 01:03
@victorlin victorlin self-assigned this May 24, 2024
@nextstrain-bot nextstrain-bot temporarily deployed to nextstrain-s-victorlin--vtah4q May 24, 2024 01:04 Inactive
This is generated upon running tsc.
@victorlin victorlin temporarily deployed to nextstrain-s-victorlin--vtah4q May 24, 2024 01:57 Inactive
@victorlin victorlin mentioned this pull request May 24, 2024
2 tasks
static-site/tsconfig.json Show resolved Hide resolved
static-site/tsconfig.json Show resolved Hide resolved
@victorlin victorlin requested a review from a team May 25, 2024 00:32
Copy link
Member

@jameshadfield jameshadfield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code passes type checks - both npx next build static-site and cd static-site && npx tsc --noEmit. Running the server and/or building the static-site doesn't result in modifications to the tsconfig from my testing. Mirroring the Auspice config is a nice move.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you find a way to get this working with VS-Code @victorlin?

I can get VS-Code to use the workspace's tsc just fine (v4.9.3) but not to use this config file. According to microsoft/vscode#12463 this is a known bug and won't be fixed.

Specifically, in the current code I get errors in _app.tsx because the default config doesn't use "jsx": "preserve":

image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What you saw is because that file wasn't included in the TSConfig:

"include": [
"next-env.d.ts",
"src/**/*",
],

This should be fixed now with a7bd07c – can you try again?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - thanks! I was (literally) looking in the wrong place 👍

(VS-Code's TypeScript output logs are pretty sparse, doesn't help debugging much...)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which output logs are you referring to? So far I'm using it to look for rule violations (i.e. "problems", red squiggles) which seems sufficient. For example, when enabling noImplicitAny on #874, I can see all the violations in static-site/src/components/ListResources/Modal.tsx.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I mean the debug/logging output from the typescript checker VS-Code is running itself, which is how I've debugged other things not working (linters, syntax highlighting etc)

image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh gotcha! It looks like that log only contains info on the TS Server process. There is a separate log that you can enable/view using the command TypeScript: Open TS Server log. It's much more verbose (6MB large from just a couple minutes of usage) but not sure if it would've been helpful in this case.

static-site/src/ was mirrored from Auspice, but the include filepaths
should be project-specific. pages/ contains source files for this
project and should be added.
@victorlin victorlin temporarily deployed to nextstrain-s-victorlin--vtah4q May 28, 2024 17:31 Inactive
@victorlin victorlin merged commit 68264db into master May 28, 2024
7 checks passed
@victorlin victorlin deleted the victorlin/configure-static-site-typescript branch May 28, 2024 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants