Throw error when SP URL is `null`
Closed this issue ยท 6 comments
While testing a command I used a variable to store my site URL so it didn't clutter my command args too much. However, I was getting a strange error as shown below:
This error was due to the fact that I made a typo and --webUrl
was null
(because the variable I specified didn't exist).
This is because the validator we are using is as follows:
cli-microsoft365/src/utils/validation.ts
Lines 349 to 360 in 3152fc7
In the command (and other commands) we validate it like this:
cli-microsoft365/src/m365/spo/commands/file/file-add.ts
Lines 139 to 142 in 3152fc7
Shouldn't we output an error message when the value is null
? The current error Error: false
doesn't help at all.
yes we should ๐.
also we should double-check this behavior in other shells. in bash
probably this would be just an empty string. I don't know about other.
luckily we usually use isValidSharePointUrl
for this validation so I hope this would only require us to do some refactoring in this single place
Yes, that's the importance of #initTypes
right now. We should type all options as much as possible. If we don't, Minimist will decide what type a variable is. When you pass --webUrl null
, it translates to --webUrl
which is a flag. In that case, it will get the value true
, and .indexOf
is not a function of a boolean value. We have not paid enough attention to this in the past.
Bummer, what if we create an issue to fix this problem globally and add the missing #initTypes?
It's a lot of work since this applies to every single option in every single command. The same thing will happen when you run entra m365group get --id $iDoNotExist
.
I know this is a known issue, and I try to fix this gradually in every command I work on/review.
The good news is, with the implementation of ZOD, this will all be fixed because every option will be typed automatically. So I suggest that we just deal with it right now. Otherwise, we have to fix this in 90% of all commands right now, and we still have to implement ZOD in every command, which will fix it anyway. So that's kind of (a lot of) double work.
Can I work on it?