-
Notifications
You must be signed in to change notification settings - Fork 363
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
Add TypeScript definitions. #269
Conversation
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.
Nice work! 🎉
e6c6e3b
to
408481f
Compare
@ryan-roemer realized I was type checking the CLI options, not the Dashboard options 🤦♂️. Pinging for re-review if you have a second! |
@@ -0,0 +1 @@ | |||
dist-* |
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.
is this still needed?
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.
Yep – this is precautionary. When a user runs yarn dev
it'll generate the bundle.js
file for the built fixture (whichever one they use). Rather than relying on a user to delete that file in order to pass local linting scripts, I figured we'd just tell ESLint to ignore these files altogether. Does that make sense?
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.
Sounds good!
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.
One comment/question, otherwise LGTM!
This PR adds basic TypeScript definitions to
webpack-dashboard
. Since our API surface is so small – it essentially consists of the possible options you can pass to theDashboard
plugin – this is a fairly minor patch. It's really intended for those souls who really, really want to write awebpack.config.ts
. I'd ideally like to roll this and theinspectpack
upgrade together into a 2.1.0 release if that sounds good to you @ryan-roemer.Some highlights:
color
passed toDashboard
to ensure that it's a supported ANSI color as per the colors outlined here: https://github.com/chalk/ansi-styles#colors. These are the colors supported bychalk
, but I noticed that the*-Bright
variants don't get picked up 😞. The ones included are confirmed to work.index.d.ts
file incheck
andcheck-ci
commands.webpack.config.ts
file and ensuringtsc
caught the errors. We'll want to confirm that this holds true after publishing.This would close #229.