Feature Request: portalLib-functions in XP8 should throw instead of returning null

Features
1

Throwing instead of returning null

From an application programmer – who uses TypeScript – standpoint, I think that some of the functions in portalLib would be improved if they threw an Exception instead of returning null.

  • getContent()
  • getComponent()
  • getSite()
  • getSiteConfig()
  • getIdProviderKey()

If these functions are used where they are intended (page, part etc.). they will always return something.

If they are used where they are not intended to be used (main.js or in a task), they will return null today.

Problem:

The current situation is not “developer experience optimized” for the happy path. In every part we create, we need to check that the results of the functions are not null – or use ! to assert that a value is not null (which is considered bad practice).

Solution:

Since this is a breaking change this suggestion is intended for XP8.

By making the functions throw an Exception instead of returning null, we can drop a bunch of boilerplate to handle null from our code (pretty much every part/layout/page).

And the functions are used in main.js or a task – maybe it’s just as well to fail it early, since these values are not available there anyway.

– Tom Arild

2

For:

I like the idea of getting better error messages, null really doesn’t say anything about what went wrong.

Against:

In terms of syntax though

if (truthy) {
    ...
}

is shorter than

try {
   ...
} catch (e) {
  // no-op
}

Sidenote:
! casts a truthy value to boolean false

3

I don’t see try/catch needing to be used much in practice. If you are using getContent() in a context where it throws, you are probably using it incorrectly and should be doing something different than try/catch.

4

If you don’t need the try/catch, then you don’t need the if either…

5

I disagree. You don’t always need to catch Exceptions, sometimes the correct thing is letting something higher up catch it.

In the case of a part controller, getContent() should never throw an Exception. But, if an Exception is thrown for some reason, the correct solution (in my applications) is letting XPs error handling take care of it, not handling it in every part.

Today when getContent() returns Hit | null then I either need an if statement to confirm that the value is there, or ! (Non-Null Assertion Operator which is considered bad practice).

6

Aha, I knew typings had a role to play in this somewhere :slight_smile:

7

I’ve passed it on to the “stakeholders”.

1 Like
8

We have discussed this. We see that throw could provide a more detailed reason for the error, but we will not change the default behaviour now:

a) It is highly likely that for instance getContent() will return null. I.e. content not published, insufficient permissions etc etc, so handling this at a higher level is rarely sufficient to deal with the situation. You would often have to add a whole lot of try/catch over checking for null.
b) You can always create your own helper functions where you throw an error if value is null
c) It will require a major rewrite for people upgrading to XP8