fix: add strict null checks #330
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
@W-12220467@ changed ref plugin to plugin-auth
Closed
|
@mshanemc made some additional changes beyond suggestions. Have another look please. |
mshanemc
approved these changes
Apr 28, 2023
There was a problem hiding this comment.
approved because it's leaving it better than it was.
JsonMap has the same problem Record<string, unknown>. "oh, any object is fine!". The purpose of raising the TS bar is not to make it compile, but to tighten up our types to find/prevent mistakes.
Vague types have their uses (plugin-data: oh, it's a json file that we don't know anything about because it's custom objects from their org). But in this case, we know what oclif plugin interface look like, etc. And the commandMeta type we own
| private pluginMap(plugins: string[]): JsonMap { | ||
| const pluginToParentPlugin: JsonMap = {}; | ||
| private pluginMap(plugins: string[]): Record<string, unknown> { | ||
| const pluginToParentPlugin: Record<string, unknown> = {}; |
There was a problem hiding this comment.
aren't the values always string? why does it need unknown?
| const plugins: JsonMap = {}; | ||
| const topicsMeta: JsonMap = {}; | ||
| private async loadTopicMetadata(): Promise<Record<string, unknown>> { | ||
| const plugins: Record<string, unknown> = {}; |
There was a problem hiding this comment.
aren't this string, boolean?
There was a problem hiding this comment.
other question--what is this const plugins doing? I see it get modified below, but it never leaves the method or persists anything, right?
There was a problem hiding this comment.
@mshanemc it is used in the command for loop to prevent plugin topics from being loaded more than one.
| @@ -230,7 +224,7 @@ export default class CommandReferenceGenerate extends SfCommand<AnyJson> { | |||
| return command.load.constructor.name === 'AsyncFunction' ? await command.load() : command.load(); | |||
| } | |||
|
|
|||
| private loadCliMeta(): JsonMap { | |||
| private loadCliMeta(): Record<string, unknown> { | |||
There was a problem hiding this comment.
if it has 3 known keys of known type, why use a vague return type?
mshanemc
added a commit
that referenced
this pull request
May 30, 2023
* Updates for `sf` (#34) * feat: support sf BREAKING CHANGE: only works for sf now * test: update to work with new file structure * refactor: use oclif/core * chore: bump to 2.0.0 * chore: satisfy dev-scripts * chore: code review * chore: regenerate yarn.lock * chore(release): 2.0.0 [ci skip] * fix: use this.config * chore(release): 2.0.1 [ci skip] * chore: update circle config for v2 [skip ci] * fix: update generate command reference * chore(release): 2.0.2 [ci skip] * fix: populate topic command xml * chore(release): 2.0.3 [ci skip] * fix: append suffix to all filenames * test: update filenames * chore(release): 2.0.4 [ci skip] * fix: use unique id * chore(release): 2.0.5 [ci skip] * fix: don't generate xml for topics * chore(release): 2.0.6 [ci skip] * fix: improve error handling * chore(release): 2.0.7 [ci skip] * fix: update template files for sf * update help file for "sf" * update sf page that lists the plugin versions * tweaks to command topics for "sf" * tweaks to topic ditamaps for "sf" * fix: update test with new title in command XML file * chore(release): 2.0.8 [ci skip] * fix: use latest sf-plugins-core * chore(release): 2.0.9 [ci skip] * fix: change title/shortdesc of main ditamap so it says "sf" * chore(release): 2.0.10 [ci skip] * fix: stop generating main topic intro * chore(release): 2.0.11 [ci skip] * fix: bump to GA deps (#37) * chore(release): 2.0.12 [ci skip] * feat: add --all (#38) * chore(release): 2.1.0 [ci skip] * chore: fix and run windows unit tests * chore(release): 2.1.1 [ci skip] * feat: update dependencies * chore: add main to package.json * chore(release): 2.2.0 [ci skip] * fix: dont run default functions * chore(release): 2.2.1 [ci skip] * fix: remove cli-ux reference * chore(release): 2.2.2 [ci skip] * fix: prevent empty tags * chore(release): 2.2.3 [ci skip] * fix: escape tags * chore(release): 2.2.4 [ci skip] * fix: remove duplicates * fix: refine default value * chore(release): 2.2.5 [ci skip] * fix: remove async handlebars * chore(release): 2.2.6 [ci skip] * fix: default value * chore(release): 2.2.7 [ci skip] * chore: remove .tgz [skip ci] * fix: remove console.log * chore(release): 2.2.8 [ci skip] * fix: improve types (#99) * fix: improve types * chore: regenerate yarn.lock * fix: description no longer required for sf @W-12296410@ * Revert "fix: description no longer required for sf" This reverts commit 1a4774c. * fix: description no longer required for sf @W-12296410@ * fix: add GHA folder @W-0@ * chore: changes * chore: apply requested changes * chore(release): 2.2.9 [skip ci] * fix: check if plugin exists * chore(release): 2.2.10 [skip ci] * feat: use oclif/core v2 * chore(release): 2.3.0 [skip ci] * fix: duplicate flag descriptions * chore(release): 2.3.1 [skip ci] * feat: jit install command (#265) * feat: add jit install command * chore: simplify implementation * chore(release): 2.4.0 [skip ci] * fix: jit install * chore(release): 2.4.1 [skip ci] * fix: add strict null checks @W-12220467@ changed ref plugin to plugin-auth * chore: remove unneeded case * refactor: rid project of fs-extra * chore: apply review suggestions * refactor: get rid of json map * chore: apply changes from review * chore(release): 2.4.2 [skip ci] * Revert "Merge pull request #330 from salesforcecli/phale/strict-null-checks" This reverts commit 5e0240f, reversing changes made to d730f40. # Conflicts: # package.json * fix: revert 2.4.2 * chore(release): 2.4.4 [skip ci] * fix: render cmd and flag sum/desc ejs template (#350) * chore(release): 2.4.5 [skip ci] * fix: don't overwrite commands with equivalent topics in other plugins * chore: better progress * chore: wip * refactor: narrower types and stronger type checking @W-13138789@ * chore: fix tests after merge * chore: reapply stash * chore: wireit updates * test: e2e tests generate test files first * refactor: typeguard in place of assertion * fix: binary is on commandMeta, not command * fix: merge subtopics, use maps * fix: config replacements in descriptions * fix: use the correct ensureArray * docs: readme * refactor: reduce ts workarounds * chore: remove unused code * fix: missing commands where there was a topic collision * chore: gitignore for my output files * refactor: remove unused --------- Co-authored-by: Peter Hale <[email protected]> * chore(release): 2.4.6 [skip ci] * ci: merge v2 into main * chore: bye, circle * style: remove comments * fix: misapplied change from merge * chore: remove commen --------- Co-authored-by: Mike Donnalley <[email protected]> Co-authored-by: SF-CLI-BOT <[email protected]> Co-authored-by: Juliet Shackell <[email protected]> Co-authored-by: Rodrigo Espinosa de los Monteros <[email protected]> Co-authored-by: Peter Hale <[email protected]> Co-authored-by: peternhale <[email protected]> Co-authored-by: Willhoit <[email protected]> Co-authored-by: svc-cli-bot <[email protected]> Co-authored-by: Cristian Dominguez <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@W-12220467@
changed ref plugin to plugin-auth