chore(jsdoc): document @stencil/sass #225
Conversation
| @@ -35,7 +46,7 @@ export function loadDiagnostic(context: d.PluginCtx, sassError: SassException, f | |||
|
|
|||
| if (errorLineIndex > -1) { | |||
| try { | |||
| const sourceText = context.fs.readFileSync(diagnostic.absFilePath) as string; | |||
| const sourceText = context.fs.readFileSync(diagnostic.absFilePath); | |||
There was a problem hiding this comment.
readFileSync returns a string, this assertion is unneeded
| * @param input the numeric error code to convert | ||
| * @returns the stringified error code | ||
| */ | ||
| function formatCode(input: number): string { |
There was a problem hiding this comment.
while this shouldn't happen per the type system, I left the nullish check in here since this was largely a documentation effort
There was a problem hiding this comment.
I think since we don't have "strict": true in this project input could be called with undefined and TypeScript wouldn't let us know, so the check might be wise. Although to be fair, just inlining String(num ?? "") would accomplish the same thing as this function anyway
| * @param input the numeric error code to convert | ||
| * @returns the stringified error code | ||
| */ | ||
| function formatCode(input: number): string { |
There was a problem hiding this comment.
I think since we don't have "strict": true in this project input could be called with undefined and TypeScript wouldn't let us know, so the check might be wise. Although to be fair, just inlining String(num ?? "") would accomplish the same thing as this function anyway
| * | ||
| * @param opts options to configure the plugin | ||
| * @return the configured plugin | ||
| */ | ||
| export function sass(opts: d.PluginOptions = {}): d.Plugin { |
There was a problem hiding this comment.
Am I right that this is the function people will import into their Stencil config if they want to use sass? If that's right, it might be good to adorn this jsdoc with just a few more helpful things, like maybe just a link to the README (https://github.com/ionic-team/stencil-sass) and possibly to the plugin documentation (https://stenciljs.com/docs/plugins) although the latter is a bit spare at the moment. I'm just thinking that that way when you hover over the symbol in your editor you get some sort of a friendly message in the JSDoc.
this commit adds jsdoc to the @stencil/sass package to help with transferring knowledge about the package/making it easier to pick up to work on. this is being done prior to the upgrade to sass 1.58.0, which will lead to type changes within the project. this commit is the result of the exercise of the author refamiliarizing themself with the project a bit, rather than a direct dependency of the aforementioned update.
rwaskiewicz
force-pushed
the
rwaskiewicz/doc-pkg
branch
from
2f22174
to
defb8fb
Compare
Pull request checklist
Please check if your PR fulfills the following requirements:
npm run build) was run locally and any changes were pushednpm test) were run locally and passednpm run prettier) was run locally and passedPull request type
Please check the type of change your PR introduces:
What is the current behavior?
This package is largely undocumented
GitHub Issue Number: N/A
What is the new behavior?
this commit adds jsdoc to the @stencil/sass package to help with transferring knowledge about the package/making it easier to pick up to work on. this is being done prior to the upgrade to sass 1.58.0, which will lead to type changes within the project. this commit is the result of the exercise of the author refamiliarizing themself with the project a bit, rather than a direct dependency of the aforementioned update.
Does this introduce a breaking change?
Testing
The formatter & type checker should continue to pass (only "functional" changes were explicit types added)