-
Notifications
You must be signed in to change notification settings - Fork 76
Add build step using TypeScript #120
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
Conversation
7abb348 to
e591a1a
Compare
| "description": "A grafana dashboard generator", | ||
| "main": "index.js", | ||
| "main": "grafana/index.js", | ||
| "types": "grafana/index.d.ts", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem to point to anything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see grafana is generated by ts with npm run prepublish?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, the grafana is to be generated by tsc on prepublish
package.json
Outdated
| "@types/jest": "^30.0.0", | ||
| "@types/node": "^24.9.1", | ||
| "@types/node-fetch": "^2.6.13", | ||
| "@types/tape": "^5.8.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't look like we're using tape anymore so can we remove @types/tape?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, let me remove it
| @@ -0,0 +1,10 @@ | |||
| { | |||
| "extends": "./tsconfig.json", | |||
| // Visit https://aka.ms/tsconfig to read more about this file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
json doesn't support comments like these. Can you confirm that the ts is interpreting tsconfig.build.json and tsconfig.json as JS instead of JSON?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tsconfig is actually using JSONC - this comment, btw, and most of the config here, was generated by running tsc --init
Introducing a build step before package publishing, so sources can be updated to use more modern syntax and eventually migrate to TypeScript.
Changes:
grafanafolder tosrcgrafanaas output target for tsc output, this way - paths for published files would remain unchanged, ensuring backward compatibilityindex.jsexport to part ofsrcfolder, updatingindex.jsat package root to point tografana/index.js(so it is backward compatible if file happens to be used directly)d.tsoutput in tsconfig - it is to be enabled separately when actually converting sources to TypeScript