[Critical Bug] Changes to JSONObject in [email protected] are breaking jupyterlab extension builds

[telamonian]

If I try to build the latest master of jupyter-fs:

git clone [email protected]:jpmorganchase/jupyter-fs.git
cd jupyter-fs
make js

I get the following error on the tsc (ie typescript compilation) step:

$ tsc
../node_modules/@jupyterlab/observables/lib/modeldb.d.ts:227:9 - error TS2416: Property 'changed' in type 'ObservableValue' is not assignable to the same property in base type 'IObservableValue'.
  Type 'ISignal<this, IChangedArgs>' is not assignable to type 'ISignal<IObservableValue, IChangedArgs>'.
    Type 'this' is not assignable to type 'IObservableValue'.
      Type 'ObservableValue' is not assignable to type 'IObservableValue'.
        The types returned by 'get()' are incompatible between these types.
          Type 'JSONValue' is not assignable to type 'PartialJSONValue'.
            Type 'JSONObject' is not assignable to type 'PartialJSONValue'.
              Type 'JSONObject' is not assignable to type 'string'.

227     get changed(): ISignal<this, ObservableValue.IChangedArgs>;
            ~~~~~~~

../node_modules/@jupyterlab/observables/lib/modeldb.d.ts:231:5 - error TS2416: Property 'get' in type 'ObservableValue' is not assignable to the same property in base type 'IObservableValue'.
  Type '() => JSONValue' is not assignable to type '() => PartialJSONValue'.

231     get(): JSONValue;
        ~~~


Found 2 errors.

This is definitely being caused by the latest release of @lumino/coreutils, since if I add a resolution to the previous coreutils to my package.json:

    "resolutions": {
        "@lumino/coreutils": "1.5.0"
    }

the build error is fixed.

I beleive the underlying cause is the redefintion of JSONObject that was part of the 1.5.1 release.

@lumino/[email protected]:

lumino/packages/coreutils/src/json.ts

Lines 26 to 30 in 00ae441

	/**
	* A type definition for a JSON object.
	*/
	export
	interface JSONObject { [key: string]: JSONValue; }

@lumino/[email protected]:

lumino/packages/coreutils/src/json.ts

Lines 26 to 30 in 36debf6

	/**
	* A type definition for a JSON object.
	*/
	export
	interface JSONObject { [key: string]: JSONValue | ReadonlyJSONValue; }

Because this was a patch release of coretuils (1.5.0 => 1.5.1), you'll likely only see the bug if you delete your yarn.lock (or if start from a fresh build that never had one).

[telamonian]

pinging @blink1073 @GordonSmith

[blink1073]

Yeah we should revert that change and make a 1.5.2

[telamonian]

In his commit message for the [email protected] upgrade, @GordonSmith implied that the change to JSONObject had to do with microsoft/TypeScript#36696.

So it sounds like the revert will have to remove the entirety of 36debf6/the typescript 3.9 upgrade

[telamonian]

I've added a PR that reverts 36debf6 over at #84

[telamonian]

It would be nice if we could keep the typescript upgrade, so I've now also added a minimal PR (over at #85) that just reverts the changes to JSONObject while leaving the rest of 36debf6 intact. Mostly just to see if it will pass the CI tests

update

if it will pass the CI tests

It does not. See details at #85 (comment).

@GordonSmith Unless you have a quick and easy fix for this, I think our only reasonable option is to revert 36debf6 in full (by pulling in #84) and then do a release ASAP

@blink1073 Do you agree?