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

Get the site building on node 16 #2473

Merged
merged 13 commits into from Feb 7, 2023
Merged

Get the site building on node 16 #2473

merged 13 commits into from Feb 7, 2023

Conversation

orta
Copy link
Contributor

@orta orta commented Aug 6, 2022

Removes all the ESM imports for JSON files because different versions of node require different syntax there ( import asserts in 16, none in 14)

@orta orta mentioned this pull request Nov 30, 2022
@andrewbranch
Copy link
Member

14 will be EOL in half a year; do you have strong feelings against making 16 the minimum?

@orta
Copy link
Contributor Author

orta commented Nov 30, 2022

Yep, totally reasonable IMO

@orta
Copy link
Contributor Author

orta commented Dec 17, 2022

This PR should get merged

@andrewbranch
Copy link
Member

I’d rather just add the import assertions than switch to readFile

@orta
Copy link
Contributor Author

orta commented Jan 14, 2023

Meh, I'm not really motivated in moving it to another syntax which could also break instead of being fully backwards compat

@orta
Copy link
Contributor Author

orta commented Feb 6, 2023

import assertions went back a TC39 stage, dropping it is still the right call imo

@andrewbranch
Copy link
Member

Yep, now I agree with you 😅

@andrewbranch
Copy link
Member

Going to retrigger CI

@andrewbranch andrewbranch reopened this Feb 6, 2023
@andrewbranch
Copy link
Member

Any idea what the build error is?

@orta
Copy link
Contributor Author

orta commented Feb 7, 2023

Looks like the bootstrap command which grabs the release info is crashing,

➤ YN0000: [@typescript/sandbox]: (node:3775) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 2)

Couldn't say why, and it didn't repro locally, but maybe an old version of axios which doesn't run on Node 16? Either way I switched it out to node-fetch which I trust more

@orta
Copy link
Contributor Author

orta commented Feb 7, 2023

Just rebased: would recommend against doing these sorts of build steps:

    - run: |
          node test/verifyPackageVersions.js
          yarn docs-sync pull microsoft/TypeScript-Website-localizations#main 1
          yarn bootstrap
          yarn build
          yarn build-site
          cp -r packages/typescriptlang-org/public site

Makes it harder to see which step failed unless you echo titles etc

@orta
Copy link
Contributor Author

orta commented Feb 7, 2023

cool, looks like we have a repo for the canvas bail - will look later

@andrewbranch
Copy link
Member

Yeah, that was the one that was angry in #2680

@orta
Copy link
Contributor Author

orta commented Feb 7, 2023

Alright, this looks ready - now I know that the CI should turn out red, but that is (now) expected.

What's happening is that:

      - name: "Validates that TypeScript plugins work"
        run: |
          cd ..
          npm init typescript-playground-plugin playground-my-plugin

Runs against v2 not the current files:

$ rollup -c rollup.config.js

src/index.ts → dist...
[!] (plugin typescript) Error: @rollup/plugin-typescript TS2792: Cannot find module 'lz-string'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' option?
src/vendor/typescript-vfs.d.ts (57:140)

57 export declare const createDefaultMapFromCDN: (options: CompilerOptions, version: string, cache: boolean, ts: TS, lzstring?: typeof import("lz-string"), fetcher?: typeof fetch, storer?: typeof localStorage) => Promise<Map<string, string>>;

eb65fda fixes this

As it is the last item in the CI, then that validates that the rest of the system is 👍🏻

@orta
Copy link
Contributor Author

orta commented Feb 7, 2023

If we get this in and it's all fine, I'll take a look at bumping node to 18, and updating gatsby to 5.x - so that there's a good few more years runway

@andrewbranch andrewbranch merged commit 4a670b3 into microsoft:v2 Feb 7, 2023
@andrewbranch
Copy link
Member

Thanks Orta!

@orta
Copy link
Contributor Author

orta commented Feb 8, 2023

Confirming that this all seems to look good on https://www.staging-typescript.org/

@andrewbranch
Copy link
Member

Merges to v2 go straight to prod these days 😉

@orta
Copy link
Contributor Author

orta commented Feb 9, 2023

nice!

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.

None yet

2 participants