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

refactor: use stricter typescript #3542

Merged
merged 10 commits into from
Dec 11, 2024
Merged

refactor: use stricter typescript #3542

merged 10 commits into from
Dec 11, 2024

Conversation

jasonkuhrt
Copy link
Collaborator

@jasonkuhrt jasonkuhrt commented Dec 11, 2024

This PR leverages community tsconfig strict settings and updates codebase
to comply with it. All code changes are minor. Most fall into one of these
categories:

  1. Record index access syntax.
  2. Handling of undefined states (sometimes actually improved, sometimes
    just needing a ! to help TS).

Pedantically, a patch release would be needed for affected packages. I would suggest ignoring these changes in our changelog nor making a release.

Copy link

changeset-bot bot commented Dec 11, 2024

⚠️ No Changeset found

Latest commit: eff57e7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

github-actions bot commented Dec 11, 2024

Apollo Federation Subgraph Compatibility Results

Federation 1 Support Federation 2 Support
_service🟢
@key (single)🟢
@key (multi)🟢
@key (composite)🟢
repeatable @key🟢
@requires🟢
@provides🟢
federated tracing🟢
@link🟢
@shareable🟢
@tag🟢
@override🟢
@inaccessible🟢
@composeDirective🟢
@interfaceObject🟢

Learn more:

Copy link
Contributor

github-actions bot commented Dec 11, 2024

✅ Benchmark Results

     ✓ no_errors{mode:graphql}
     ✓ expected_result{mode:graphql}
     ✓ no_errors{mode:graphql-jit}
     ✓ expected_result{mode:graphql-jit}
     ✓ no_errors{mode:graphql-response-cache}
     ✓ expected_result{mode:graphql-response-cache}
     ✓ no_errors{mode:graphql-no-parse-validate-cache}
     ✓ expected_result{mode:graphql-no-parse-validate-cache}
     ✓ no_errors{mode:uws}
     ✓ expected_result{mode:uws}

     checks.......................................: 100.00% ✓ 517228     ✗ 0     
     data_received................................: 2.1 GB  14 MB/s
     data_sent....................................: 104 MB  693 kB/s
     http_req_blocked.............................: avg=1.49µs   min=972ns    med=1.31µs   max=279.99µs p(90)=1.98µs   p(95)=2.16µs  
     http_req_connecting..........................: avg=2ns      min=0s       med=0s       max=133.48µs p(90)=0s       p(95)=0s      
     http_req_duration............................: avg=361.62µs min=221.2µs  med=329.7µs  max=14.76ms  p(90)=469.07µs p(95)=492.27µs
       { expected_response:true }.................: avg=361.62µs min=221.2µs  med=329.7µs  max=14.76ms  p(90)=469.07µs p(95)=492.27µs
     ✓ { mode:graphql-jit }.......................: avg=291.35µs min=221.2µs  med=274.48µs max=14.72ms  p(90)=305.88µs p(95)=322µs   
     ✓ { mode:graphql-no-parse-validate-cache }...: avg=492.36µs min=401.69µs med=470.37µs max=6.01ms   p(90)=515.52µs p(95)=558.76µs
     ✓ { mode:graphql-response-cache }............: avg=345.99µs min=270.92µs med=329.81µs max=6.33ms   p(90)=359.85µs p(95)=370.47µs
     ✓ { mode:graphql }...........................: avg=367.88µs min=280.85µs med=338.66µs max=14.76ms  p(90)=395.69µs p(95)=450.44µs
     ✓ { mode:uws }...............................: avg=346µs    min=271.72µs med=328.02µs max=5.95ms   p(90)=363.84µs p(95)=385.84µs
     http_req_failed..............................: 0.00%   ✓ 0          ✗ 258614
     http_req_receiving...........................: avg=32.7µs   min=15.93µs  med=32.42µs  max=2.84ms   p(90)=39.02µs  p(95)=41.67µs 
     http_req_sending.............................: avg=8.6µs    min=5.9µs    med=7.56µs   max=343.53µs p(90)=10.96µs  p(95)=12µs    
     http_req_tls_handshaking.....................: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting.............................: avg=320.31µs min=191.87µs med=289.24µs max=14.63ms  p(90)=427.29µs p(95)=449.12µs
     http_reqs....................................: 258614  1724.07085/s
     iteration_duration...........................: avg=575.03µs min=387.22µs med=539µs    max=15.4ms   p(90)=686.45µs p(95)=714.84µs
     iterations...................................: 258614  1724.07085/s
     vus..........................................: 1       min=1        max=1   
     vus_max......................................: 2       min=2        max=2   

Copy link
Contributor

github-actions bot commented Dec 11, 2024

💻 Website Preview

The latest changes are available as preview in: https://719ce521.graphql-yoga.pages.dev

@jasonkuhrt jasonkuhrt requested a review from ardatan December 11, 2024 17:17
tsconfig.json Show resolved Hide resolved
tsconfig.json Show resolved Hide resolved
tsconfig.json Show resolved Hide resolved
tsconfig.json Outdated Show resolved Hide resolved
@jasonkuhrt jasonkuhrt requested a review from ardatan December 11, 2024 18:44
"resolveJsonModule": true,
"skipLibCheck": true,
"jsx": "preserve",
"exactOptionalPropertyTypes": false,
Copy link
Collaborator Author

@jasonkuhrt jasonkuhrt Dec 11, 2024

Choose a reason for hiding this comment

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

I do like this one but its worth its own PR, a lot of code currently conflates this which is pretty typical with this settings when trying it out for the first time on a codebase.

@@ -5,16 +5,26 @@ import { minify as minifyT } from 'html-minifier-terser';

const __dirname = path.dirname(fileURLToPath(import.meta.url));

// TODO review if this function is still needed.
Copy link
Collaborator

@ardatan ardatan Dec 11, 2024

Choose a reason for hiding this comment

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

Typings might be wrong because if I remember correctly, it returns Buffer which has toString method that accepts an encoding type.

@jasonkuhrt

This comment was marked as outdated.

@ardatan ardatan merged commit ebec485 into main Dec 11, 2024
24 checks passed
@ardatan ardatan deleted the refactor/strict-typescript branch December 11, 2024 22:34
@ardatan
Copy link
Collaborator

ardatan commented Dec 11, 2024

Awesome! Thanks 🙏

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.

2 participants