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

Typescript Support #137

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Typescript Support #137

wants to merge 4 commits into from

Conversation

bdbch
Copy link
Member

@bdbch bdbch commented Jun 16, 2023

This PR adds Typescript support & Type Definitions for all dotaconstant values.

"build": "npm run build:patch && npm run build:data && npm run build:ts",
"build:ts": "rimraf dist && tsc --project tsconfig.json",
"build:data": "node tasks/updateconstants",
"build:patch": "node tasks/updateconstants"
Copy link
Member

Choose a reason for hiding this comment

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

should be patch script?

region: require(__dirname + "/build/region.json"),
skillshots: require(__dirname + "/build/skillshots.json"),
xp_level: require(__dirname + "/build/xp_level.json")
abilities: require(__dirname + "/src/data/abilities.json"),
Copy link
Member

Choose a reason for hiding this comment

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

Do we need both this and the ts index file?

@@ -36,7 +36,7 @@ const extraStrings = {
DOTA_UNIT_TARGET_HERO: "Hero",
DOTA_UNIT_TARGET_BASIC: "Basic",
DOTA_UNIT_TARGET_BUILDING: "Building",
DOTA_UNIT_TARGET_TREE: "Tree"
DOTA_UNIT_TARGET_TREE: "Tree",
Copy link
Member

Choose a reason for hiding this comment

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

I'd put the formatting only changes in a separate PR to make this easier to review

@howardchung
Copy link
Member

howardchung commented Jun 17, 2023

Looks like this combines the build and json directories into source files and outputs into dist? I feel like I'd miss the ability to diff the generated output since dist is gitignored

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