Strict null checks for coreutils

[vidartf]

References

Partially supersedes #7002.

Code changes

Some updated typings to be more clear about where null/undefined is expected/allowed, some bug fixes.

User-facing changes

None.

Backwards-incompatible changes

Strictly, these could all be categorized as bug fixes, but due to the number of minor tweaks, it might be better to do this as part of a major version release.

@afshin If this causes a major version release, it might be the timing for moving the settings registry etc, out of coreutils.

TODO before merging

• Some bugs were left in place as I want input on how to resolve. These should be fixed before merging.
• Individual questions below should all be resolved.
• Each type in the public facing API in the package should be re-evaluated if it should also accept null/undefined... or we can leave it as is, and issue bug fixes if we need to expand the accepted values to include undefined/null. (this should be done at the end of all strict null PRs)

[jupyterlab-dev-mode]

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link:

[vidartf]

@afshin There are multiple compile errors for the settings registry that I left in place. Would you mind having a look at those to figure out what the correct resolution is?

[jasongrout]

Strictly, these could all be categorized as bug fixes, but due to the number of minor tweaks, it might be better to do this as part of a major version release.

Great timing since the next release is a major one :)

[vidartf]

Great timing since the next release is a major one :)

It was never a guarantee that the coreutils package would get a major version bump though...

[jasongrout]

It was never a guarantee that the coreutils package would get a major version bump though...

We bumped all the packages this time around:

jupyterlab/packages/coreutils/package.json

Line 3 in 0d3d6cd

"version": "4.0.0-alpha.3",

[vidartf]

@afshin Do you have time to have a look at the current compile errors for restorablepool? I had a look at one way of resolving it, but wasn't sure of the nuances in the code.

[afshin]

@vidartf This should fix it:

--- a/packages/coreutils/src/restorablepool.ts
+++ b/packages/coreutils/src/restorablepool.ts
@@ -138,7 +138,7 @@ export class RestorablePool<

       if (objName) {
         const name = `${this.namespace}:${objName}`;
-        const data = this._restore.args(obj);
+        const data = this._restore.args?.(obj);

         Private.nameProperty.set(obj, name);
         await connector.save(name, { data });
@@ -298,7 +298,7 @@ export class RestorablePool<
     Private.nameProperty.set(obj, newName);

     if (newName) {
-      const data = this._restore.args(obj);
+      const data = this._restore.args?.(obj);
       await connector.save(newName, { data });
     }

[vidartf]

Then this should be ready for review + merge and iterate for strict null checks for other packages.

[afshin]

Approved pending minor comments.

[afshin]

Inertia wins the day on the great content vs. contents debate of 2019.

Thanks, @vidartf!