Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot load MyService.tsx component when monitoringUrl is undefined #804

Closed
slim0 opened this issue May 3, 2024 · 3 comments
Closed

Cannot load MyService.tsx component when monitoringUrl is undefined #804

slim0 opened this issue May 3, 2024 · 3 comments

Comments

@slim0
Copy link
Contributor

slim0 commented May 3, 2024

Hi,

We updated onyxia with the 8.16.4 version.
In our configuration, the monitoringUrl is undefined. In that case, the UI (MyService.tsx component) is not loaded properly. We dig in a bit and it seems that it came from this assert line:

assert(monitoringUrl !== undefined);

Question: Why asserting something that can be undefined? Maybe some other variables are also impacted.

garronej added a commit that referenced this issue May 3, 2024
@garronej
Copy link
Contributor

garronej commented May 3, 2024

Sorry about that, and thanks for reporting.

In this particular case, using assertions enables enhanced type safety. It allows TypeScript to understand that if isReady is true, then helmReleaseFriendlyName, podNames, and selectedPodName are not undefined. I was a bit quick in my assertion, forgetting that monitoringUrl can be undefined in this region. I should probably use null to represent "not yet fetched" instead of using undefined. It would prevent me from making this kind of mistake again.

More generally, assertions tell TypeScript to "trust me, I know what I'm doing" in cases where the type system cannot follow. Whenever an assertion is incorrect, like here, it makes for a very easily traceable error.

@fcomte
Copy link
Contributor

fcomte commented May 31, 2024

@garronej is it fixed ? i guess yes

@garronej
Copy link
Contributor

garronej commented Jun 1, 2024

Yes it is.

I'll do a small refactor so this kind of issues may not happen again. Ref #816

@garronej garronej closed this as completed Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants