Skip to content

Conversation

@sushichan044
Copy link
Contributor

@sushichan044 sushichan044 commented May 24, 2025

The author should do the following, if applicable

  • Add tests
  • Run tests
  • bun run format:fix && bun run lint:fix to format the code
  • Add TSDoc/JSDoc to document the code

@codecov
Copy link

codecov bot commented May 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.31%. Comparing base (2805d32) to head (cd100b4).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #4161    +/-   ##
========================================
  Coverage   91.31%   91.31%            
========================================
  Files         168      168            
  Lines       10788    10788            
  Branches     3183     3036   -147     
========================================
  Hits         9851     9851            
  Misses        936      936            
  Partials        1        1            

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sushichan044
Copy link
Contributor Author

sushichan044 commented May 24, 2025

It seems an error is occurring when type checking external libraries.
While it executes successfully if skipLibCheck: true is set, this isn't the outcome we're looking for.

https://github.com/honojs/hono/actions/runs/15224335180/job/42824439319?pr=4161#step:5:7

note: This is not happening in tsc.

Details

./../node_modules/vitest/dist/chunks/reporters.6vxQttCV.d.ts:6:10 - error TS2305: Module '"/Users/sushichan044/workspace/github.com/sushichan044/honojs-hono/node_modules/vite/index"' has no exported member 'TransformResult'.

6 import { TransformResult as TransformResult$1, ServerOptions, DepOptimizationConfig, AliasOptions, UserConfig as UserConfig$1, ConfigEnv, ViteDevServer, ModuleNode } from 'vite';
           ~~~~~~~~~~~~~~~
../../node_modules/vitest/dist/chunks/reporters.6vxQttCV.d.ts:6:48 - error TS2305: Module '"/Users/sushichan044/workspace/github.com/sushichan044/honojs-hono/node_modules/vite/index"' has no exported member 'ServerOptions'.

6 import { TransformResult as TransformResult$1, ServerOptions, DepOptimizationConfig, AliasOptions, UserConfig as UserConfig$1, ConfigEnv, ViteDevServer, ModuleNode } from 'vite';
                                                 ~~~~~~~~~~~~~
../../node_modules/vitest/dist/chunks/reporters.6vxQttCV.d.ts:6:63 - error TS2305: Module '"/Users/sushichan044/workspace/github.com/sushichan044/honojs-hono/node_modules/vite/index"' has no exported member 'DepOptimizationConfig'.

6 import { TransformResult as TransformResult$1, ServerOptions, DepOptimizationConfig, AliasOptions, UserConfig as UserConfig$1, ConfigEnv, ViteDevServer, ModuleNode } from 'vite';
                                                                ~~~~~~~~~~~~~~~~~~~~~
../../node_modules/vitest/dist/chunks/reporters.6vxQttCV.d.ts:6:86 - error TS2305: Module '"/Users/sushichan044/workspace/github.com/sushichan044/honojs-hono/node_modules/vite/index"' has no exported member 'AliasOptions'.

6 import { TransformResult as TransformResult$1, ServerOptions, DepOptimizationConfig, AliasOptions, UserConfig as UserConfig$1, ConfigEnv, ViteDevServer, ModuleNode } from 'vite';
                                                                                       ~~~~~~~~~~~~
../../node_modules/vitest/dist/chunks/reporters.6vxQttCV.d.ts:6:100 - error TS2305: Module '"/Users/sushichan044/workspace/github.com/sushichan044/honojs-hono/node_modules/vite/index"' has no exported member 'UserConfig'.

6 import { TransformResult as TransformResult$1, ServerOptions, DepOptimizationConfig, AliasOptions, UserConfig as UserConfig$1, ConfigEnv, ViteDevServer, ModuleNode } from 'vite';
                                                                                                     ~~~~~~~~~~
../../node_modules/vitest/dist/chunks/reporters.6vxQttCV.d.ts:6:128 - error TS2305: Module '"/Users/sushichan044/workspace/github.com/sushichan044/honojs-hono/node_modules/vite/index"' has no exported member 'ConfigEnv'.

6 import { TransformResult as TransformResult$1, ServerOptions, DepOptimizationConfig, AliasOptions, UserConfig as UserConfig$1, ConfigEnv, ViteDevServer, ModuleNode } from 'vite';
                                                                                                                                 ~~~~~~~~~
../../node_modules/vitest/dist/chunks/reporters.6vxQttCV.d.ts:6:139 - error TS2305: Module '"/Users/sushichan044/workspace/github.com/sushichan044/honojs-hono/node_modules/vite/index"' has no exported member 'ViteDevServer'.

6 import { TransformResult as TransformResult$1, ServerOptions, DepOptimizationConfig, AliasOptions, UserConfig as UserConfig$1, ConfigEnv, ViteDevServer, ModuleNode } from 'vite';
                                                                                                                                            ~~~~~~~~~~~~~
../../node_modules/vitest/dist/chunks/reporters.6vxQttCV.d.ts:6:154 - error TS2305: Module '"/Users/sushichan044/workspace/github.com/sushichan044/honojs-hono/node_modules/vite/index"' has no exported member 'ModuleNode'.

6 import { TransformResult as TransformResult$1, ServerOptions, DepOptimizationConfig, AliasOptions, UserConfig as UserConfig$1, ConfigEnv, ViteDevServer, ModuleNode } from 'vite';
                                                                                                                                                           ~~~~~~~~~~
../../node_modules/vitest/dist/chunks/vite.Cu7NWuBa.d.ts:4:16 - error TS2671: Cannot augment module 'vite' because it resolves to a non-module entity.

4 declare module 'vite' {
                 ~~~~~~

Found 9 errors in 2 files.

Errors  Files
     8  ../../node_modules/vitest/dist/chunks/reporters.6vxQttCV.d.ts:6
     1  ../../node_modules/vitest/dist/chunks/vite.Cu7NWuBa.d.ts:4

Files:               231
Lines:            106339
Identifiers:      106177
Symbols:          420780
Types:            353057
Instantiations:  3623408
Memory used:     298464K
Memory allocs:  10663181
Parse time:       0.027s
Bind time:        0.009s
Check time:       0.675s
Emit time:        0.000s
Total time:       0.714s

@sushichan044 sushichan044 marked this pull request as ready for review May 24, 2025 06:41
@sushichan044
Copy link
Contributor Author

#4159 (comment)

@yusukebe
Copy link
Member

@sushichan044

I think we can add skipLibCheck: true because we don't want to check the types of dependencies like Vitest.

@sushichan044 sushichan044 force-pushed the ci/tsgo-type-benchmark branch from 25d8559 to 6c0191c Compare May 24, 2025 07:28
@sushichan044
Copy link
Contributor Author

@yusukebe
On second thought, since skipLibCheck: true is often used even in actual application development, that seems fine.
I'll go ahead and change the setting with another PR.

@sushichan044 sushichan044 force-pushed the ci/tsgo-type-benchmark branch from 6c0191c to fad8227 Compare May 24, 2025 22:03
@yusukebe
Copy link
Member

@sushichan044

Is this ready for review?

@sushichan044
Copy link
Contributor Author

@yusukebe Yes!

@sushichan044 sushichan044 force-pushed the ci/tsgo-type-benchmark branch from 9319536 to 9a05e7c Compare May 25, 2025 10:21
@sushichan044
Copy link
Contributor Author

sushichan044 commented May 25, 2025

As a concern, there is a possibility that the comments from the results executed with tsc and the comments from the results executed with typescript-go may conflict and not be updated correctly.
I will investigate further.

=> should be solved in f01be98

@sushichan044
Copy link
Contributor Author

sushichan044 commented May 25, 2025

@yusukebe

When Octocov updates items such as comments, it internally uses the 'repository' value set in its configuration. Therefore, for the tsc and typescript-go benchmarks, Octocov will not be able to update comments correctly unless we use separate configuration files and specify distinct 'repository' values in them.

I preferred adding more configuration files over dynamically generating YAML on CI. What are your thoughts?

@sushichan044 sushichan044 requested a review from yusukebe May 25, 2025 10:48
@sushichan044
Copy link
Contributor Author

sushichan044 commented May 25, 2025

repository: ${GITHUB_REPOSITORY}/perf-measures
repository: ${GITHUB_REPOSITORY}/perf-measures

I haven't changed the repository name in existing tsc benchmark config
because adding /tsc would make it impossible to compare with past existing metrics 

@yusukebe
Copy link
Member

@sushichan044

I preferred adding more configuration files over dynamically generating YAML on CI. What are your thoughts?

I also prefer using configuration files over generating YAML on CI.

Copy link
Member

@yusukebe yusukebe left a comment

Choose a reason for hiding this comment

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

LGTM!

@yusukebe
Copy link
Member

@sushichan044

Looks good! Let's go with this. Thanks!

@yusukebe yusukebe merged commit 1197688 into honojs:main May 26, 2025
17 checks passed
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