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

build: add typechecking to instruments and modules #8238

Merged
merged 15 commits into from
Oct 7, 2023

Conversation

Benjozork
Copy link
Member

@Benjozork Benjozork commented Oct 1, 2023

Summary of Changes

  • Adds tsc type-checking to both instruments and standalone modules, via a custom esbuild pluigin
  • Get type-checking working in iginiter
  • Enables type-checking in PR builds
  • Fix issues with type-checking
  • Fix existing type errors found

Philosophy and design compromises

Type-checking is implemented via a custom esbuild plugin which invokes tsc before actually building the instrument/module. The plugin is integrated in both the Mach bundler (for instruments) and our custom build script from our build-utils module for standalone modules.

This has a few advantages:

  • No need for discrete type-checking tasks or scripts that need to be run separately, requiring one more command to remember;
  • Requires less modifications to our build system to conditionally turn off;
  • Allows developers to locally enable type-checking for local development work if they wish, without requiring two programs to be running at once, and natively supporting watch mode.

Bringing tsc into the mix again has some drawbacks, notably:

  • It requires every instrument and standalone module to have its own tsconfig.json file. Note that this file can simply extend another tsconfig.json file.
  • It breaks some type resolutions that previously worked when using editors' built-in TS support, requiring some changes to where some types are located;
  • It removes the possibility of building a singular bundle mixing both React and FSComponent, due to tsc not taking into account tsconfig.json files in subdirectories of the built project.

Testing instructions

  • Ensure the aircraft systems and avionics still behave correctly. Of particular note to verify are the EFB and FMS LNAV/VNAV.

How to download the PR for QA

Every new commit to this PR will cause a new A32NX artifact to be created, built, and uploaded.

  1. Make sure you are signed in to GitHub
  2. Click on the Checks tab on the PR
  3. On the left side, click on the bottom PR tab
  4. Click on the A32NX download link at the bottom of the page

@Benjozork Benjozork changed the title build+ci: add typechecking to instruments and modules build: add typechecking to instruments and modules Oct 1, 2023
@Saschl
Copy link
Contributor

Saschl commented Oct 7, 2023

QA Report

Discord: saschl
Object of testing: #8238
Tier of Testing : 2
Date : 08/10/2023

Testing Process:

Full flight from EDDS to LOWW (full local rebuild).

  1. EFB functions as usual
  2. LNAV, MCDU and FMS functions as expected
  3. Everything else looks good as well.
  4. Local build works as well with typechecking enabled :)

Testing Results:
Passed

@Benjozork Benjozork merged commit 7fbfd11 into flybywiresim:master Oct 7, 2023
6 checks passed
@Benjozork Benjozork deleted the ci/ts-typechecking branch October 7, 2023 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✔️ Done
Development

Successfully merging this pull request may close these issues.

3 participants