Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(types): cast from number to string explicitly #5342

Merged
merged 1 commit into from
Nov 2, 2023
Merged

Conversation

yoshinorin
Copy link
Member

@yoshinorin yoshinorin commented Oct 31, 2023

What does it do?

fix TS2345: Argument of type 'number' is not assignable to parameter of type 'string'. error. I ignore them with @ts-ignore when I published v7.0.0.

$ npm run build

> hexo@7.0.0 build
> tsc -b

lib/hexo/multi_config_path.ts:65:28 - error TS2345: Argument of type 'number' is not assignable to parameter of type 'string'.

65   log.i('Config based on', count, 'files');
                              ~~~~~

lib/plugins/console/generate.ts:172:44 - error TS2345: Argument of type 'number' is not assignable to parameter of type 'string'.

172       log.info('%d files generated in %s', count, cyan(interval));
                                               ~~~~~
Found 2 errors.

Screenshots

N/A

Pull request tasks

  • Add test cases for the changes.
  • Passed the CI test.

- fix `TS2345: Argument of type 'number' is not assignable to parameter of type 'string'.` error
Copy link

github-actions bot commented Oct 31, 2023

@coveralls
Copy link

Pull Request Test Coverage Report for Build 6709541831

  • 2 of 2 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 99.535%

Totals Coverage Status
Change from base Build 6707257857: 0.0%
Covered Lines: 8997
Relevant Lines: 9039

💛 - Coveralls

@uiolee
Copy link
Member

uiolee commented Nov 1, 2023

what circumstances does this problem occur. I can't reproduce it.

@yoshinorin
Copy link
Member Author

@uiolee

what circumstances does this problem occur. I can't reproduce it.

Checkout master branch and just run npm run build.

// My local env
$ systeminfo
OS Name:                  Microsoft Windows 11 Pro
OS Version:               10.0.22621 N/A Build 22621

$ node -v
v20.5.1

$  tsc -v
Version 4.6.4
// chekout master branch
$ git checkout master
Switched to branch 'master'
Your branch is up to date with 'upstream/master'.

$ git branch -v --contains 
* master         f7a581ea release: v7.0.0 (#5329)
// just run build command
$ npm run build                                

> hexo@7.0.0 build
> tsc -b

lib/hexo/multi_config_path.ts:65:28 - error TS2345: Argument of type 'number' is not assignable to parameter of type 'string'.

65   log.i('Config based on', count, 'files');
``

@uiolee
Copy link
Member

uiolee commented Nov 1, 2023

I try node@20.5.1 and still can't reproduce.
But found more issue on tsc@4.6.4

tsc 4.6.4

$ npx tsc -v
Version 4.6.4

$ npm run build

> hexo@7.0.0 build
> tsc -b

lib/box/index.ts:60:3 - error TS4055: Return type of public method from exported class has or is using private name '_File'.

60   _createFileClass() {
     ~~~~~~~~~~~~~~~~

lib/hexo/index.ts:463:3 - error TS4055: Return type of public method from exported class has or is using private name 'Locals'.

463   _generateLocals() {
      ~~~~~~~~~~~~~~~

lib/hexo/router.ts:17:27 - error TS4020: 'extends' clause of exported class 'RouteStream' has or is using private name 'Readable'.

17 class RouteStream extends Readable {
                             ~~~~~~~~


Found 3 errors.

@yoshinorin yoshinorin merged commit d2cc931 into master Nov 2, 2023
@yoshinorin yoshinorin deleted the fix/type-error branch November 2, 2023 11:03
@D-Sketon
Copy link
Member

D-Sketon commented Nov 5, 2023

v4.6.4 found 3 errors
v4.9.5 found 0 error

@yoshinorin
Copy link
Member Author

v4.6.4 found 3 errors
v4.9.5 found 0 error

Can we specify which version of tsc to use in package.json or other files?

@uiolee
Copy link
Member

uiolee commented Nov 6, 2023

I think we can follow the latest version

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants