consul icon indicating copy to clipboard operation
consul copied to clipboard

ui: Fixup 2 places where we didn't default to empty string for internal URIs

Open johncowen opened this issue 3 years ago • 2 comments

Back in https://github.com/hashicorp/consul/pull/9344 we added a dev time only warning for when our uri helper received a non-string argument (so maybe undefined), this was to avoid/be conscious of errors caused by passing undefined without realising it (a common cause of bugs).

There are a couple of places where an 'empty' argument is ok, and in those places we pass a (or @potentiallyUndefined '') through to the helper.

One fix in this PR is doing that in our DataForm where passing an empty argument is sometimes ok. DataForm is reused for creating (when a value can be undefined) and updating. We'd not been in this code for a little while, but recently had the opportunity to come in here whilst chasing down another unrelated bug. This is the last place in pre-existing code where an empty argument is ok, so we can now make the decision as to whether we want this dev time warning to fail harder (we probably do) and how it should do that, but we can punt that to a future PR.

Additionally this dev time warning did exactly what we wanted and warned us of a potential problem where we were passing an undefined value through to the uri helper when we weren't expecting it, in relation to new namespace functionality (for a new feature) that we recently added. We didn't notice this initially as we were generally trying this out in 'enterprise mode', where the variable is always set to a string, whether empty or not.

This problem didn't cause any problems in the UI as we always strip out the namespace argument from our API calls when not in enterprise mode anyway. So this fix is also just to correct things 'internally'.

The change to this last one is a little bit bigger than it should be due to prettier, but it's another 1 liner.

johncowen avatar Jul 12 '21 16:07 johncowen

This pull request has been automatically flagged for inactivity because it has not been acted upon in the last 60 days. It will be closed if no new activity occurs in the next 30 days. Please feel free to re-open to resurrect the change if you feel this has happened by mistake. Thank you for your contributions.

github-actions[bot] avatar Feb 24 '22 01:02 github-actions[bot]

CLA assistant check
All committers have signed the CLA.

hashicorp-cla avatar Mar 12 '22 17:03 hashicorp-cla