Skip to content

Commit

Permalink
chore(cli): cli import code never reaches "??" operator (#28113)
Browse files Browse the repository at this point in the history
We're seeing this linter error flag: `Warning: G] The "??" operator here will always return the left operand [suspicious-nullish-coalescing]` in the yarn upgrade [task](https://github.com/aws/aws-cdk/actions/runs/6958118087/job/18935111755). 

It is because `const defaultValue = typeof resourceProps[idProp] ?? '';` never reaches the `??` since `typeof` returns a string (returns `"undefined"` when the type its looking at is `undefined`). The code in question is buggy, but also ambiguous in meaning. It could mean:

`const defaultValue = typeof (resourceProps[idProp] ?? '');`, which means that we want `defaultValue === 'string'` when `resourceProps[idProp] === undefined`. 

or

`const defaultValue = resourceProps[idProp] ? typeof resourceProps[idProp] : '';`, which means that we want `defaultValue === ''` when `resourceProps[idProp] === undefined`. 

Later on we use `defaultValue` as the condition in a ternary operator. This tells me that the latter is the correct way to interpret the code's intention, since `''` evaluates to `false`. All other options, including `'string'` and `'undefined'`, evaluate to  `true`.



----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
kaizencc authored Nov 23, 2023
1 parent 31c18f7 commit 49839b2
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion packages/aws-cdk/lib/import.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ export class ResourceImporter {
for (const idProp of idProps) {
// If we have a value from the template, use it as default. This will only be a partial
// identifier if present, otherwise we would have done the import already above.
const defaultValue = typeof resourceProps[idProp] ?? '';
const defaultValue = resourceProps[idProp] ?? '';

const prompt = [
promptPattern.replace(/%/g, chalk.blue(idProp)),
Expand Down

0 comments on commit 49839b2

Please sign in to comment.