Skip to content

Conversation

@Rudxain
Copy link

@Rudxain Rudxain commented Jul 4, 2025

I enabled type-checking and specified which version of TypeScript and Prettier are being used.

I'm considering to remove the jsconfig in favor of a simple //@ts-check directive

Copy link
Member

@TomJo2000 TomJo2000 left a comment

Choose a reason for hiding this comment

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

Just have a couple questions.

Comment on lines +25 to 24
/**@type {unknown}*/
const repos = JSON.parse(
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Thanks! I had replaced the any by unknown because there's no absolute guarantee that the input repo.json honors the schema. But I guess it's fine to make some assumptions there

binMap.set(packageName, []);
}
binMap.get(packageName).push(binary);
/**@type {string[]}*/ (binMap.get(packageName)).push(binary);
Copy link
Member

Choose a reason for hiding this comment

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

That seems like a messy way to express that type annotation.

Copy link
Author

Choose a reason for hiding this comment

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

IDK how to do non-nullish asserts in JSDoc 😅

Copy link
Member

@thunder-coding thunder-coding left a comment

Choose a reason for hiding this comment

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

I'm not a fan of package-lock.json changes being committed by contributors. I'd prefer if it is generated by one of the organization members for security reasons.

Also for the type hints, I think it's better to just switch to TypeScript than add typehints, I find them pretty annoying to deal with and would prefer switching to TS. Also if we want to switch to TS, I find this script to be too small to actually be worth it for switching to TypeScript. It's just 272 lines of JavaScript out of which 51 are comments. Would like to know about the opinion of others on this

Rudxain added 2 commits July 4, 2025 18:42
- add some assertions
- minor refactor
- add JSDoc: comments, type-annotations, and type-casts
- `npm i '@types/node@latest' prettier@latest typescript@latest`
- strict `jsconfig.json`
- add some props to `package.json`
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.

3 participants