Update typescript (#641)

* Update typescript

I am working on another pr which uses newer typescript features, so I thought I would update it first.
This pr updates to typescript 4.9 and fixes the resulting errors. It also
sets up a stricter tsconfig_base which is used by all tsconfig files and
fixes the resulting errors.

* add back funciton that maybe needed externally

* fix test typescript errors

* Cleanup TS upgrade

Reverts parameter renames, and removes tsconfig settings with the default values. Removes as error expected directives, and fixes typings in another way.

* Enforce explicit type-only imports

This forces type-only imports to be imported with `import type`

* CI py 3.6 -> 3.11

* Fix cookie secret

* Remove notebook tests

With jupyter_server 2.0, we cannot support launching with notebook. The tests can potentially be reenabled with notebook 7.0, as it is based on jupyter_server.

* Add token auth fixture

When there is a token, xsrf cookies/headers are ignored, so using it makes tests easier.

Co-authored-by: Vidar Tonaas Fauske <510760+vidartf@users.noreply.github.com>
Co-authored-by: Vidar Tonaas Fauske <vidartf@gmail.com>

.github/workflows/tests.yml

@@ -79,7 +79,7 @@ jobs:
     strategy:
       max-parallel: 4
       matrix:
-        python-version: ['3.6', '3.7', '3.8', '3.9', '3.10', '3.11']
+        python-version: ['3.7', '3.8', '3.9', '3.10', '3.11']
 
     steps:
     - uses: actions/checkout@v2

nbdime/tests/conftest.py

@@ -234,6 +234,20 @@ def app(nbdime_base_url, filespath):
     )[0]
 
 
+@fixture
+def auth_header():
+    old = os.environ.get('JUPYTER_TOKEN')
+    os.environ['JUPYTER_TOKEN'] = TEST_TOKEN
+    try:
+        yield {
+            'Authorization': 'token %s' % TEST_TOKEN
+        }
+    finally:
+        if old:
+            os.environ['JUPYTER_TOKEN'] = old
+        else:
+            del os.environ['JUPYTER_TOKEN']
+
 @fixture
 def json_schema_diff(request):
     schema_path = os.path.join(schema_dir, 'diff_format.schema.json')
@@ -438,17 +452,33 @@ def create_server_extension_config(tmpdir_factory, cmd):
     }
     config_str = json.dumps(config)
     if isinstance(config_str, bytes):
-        config_str = unicode(config_str)
+        config_str = str(config_str)
     path.join(filename).write_text(config_str, 'utf-8')
     return str(path)
 
 
-
-@fixture(scope='module', params=('notebook', 'jupyter_server'))
+# TODO: Add back 'notebook' as param when NB 7.0 is out ?
+@fixture(scope='module', params=('jupyter_server',))
 def server_extension_app(tmpdir_factory, request):
     cmd = request.param
 
-    appname = 'NotebookApp' if cmd == 'notebook' else 'ServerApp'
+    if cmd == 'notebook':
+        token_config_location = 'NotebookApp'
+    else:
+        v = None
+        try:
+            from importlib.metadata import version
+            v = version('jupyter_server')
+        except ImportError:
+            import pkg_resources
+            v = pkg_resources.get_distribution('jupyter_server').version
+        from packaging import version
+        if version.parse(v).major >= 2:
+            token_config_location = 'IdentityProvider'
+        else:
+            token_config_location = 'ServerApp'
+
+
 
     def _kill_nb_app():
         try:
@@ -470,7 +500,8 @@ def _kill_nb_app():
         sys.executable, '-m', cmd,
          '--port=%i' % port,
         '--ip=127.0.0.1',
-        '--no-browser', '--%s.token=%s' % (appname, TEST_TOKEN)],
+        '--log-level=DEBUG',
+        '--no-browser', '--%s.token=%s' % (token_config_location, TEST_TOKEN)],
         env=env)
 
     request.addfinalizer(_kill_nb_app)

nbdime/tests/test_cli_apps.py

@@ -585,7 +585,7 @@ def test_git_mergedriver(git_repo, filespath):
 
 
 @pytest.mark.timeout(timeout=3*WEB_TEST_TIMEOUT)
-def test_git_difftool(git_repo, unique_port, popen_with_terminator):
+def test_git_difftool(git_repo, unique_port, popen_with_terminator, auth_header):
     gitdifftool.main(['config', '--enable'])
     cmd = get_output('git config --get --local difftool.nbdime.cmd').strip()
 
@@ -608,7 +608,7 @@ def test_git_difftool(git_repo, unique_port, popen_with_terminator):
         r = requests.get(url + '/difftool')
         r.raise_for_status()
         # close it
-        r = requests.post(url + '/api/closetool', json={'exitCode': 0})
+        r = requests.post(url + '/api/closetool', json={'exitCode': 0}, headers=auth_header)
         r.raise_for_status()
         time.sleep(0.25)
     # wait for exit
@@ -617,7 +617,7 @@ def test_git_difftool(git_repo, unique_port, popen_with_terminator):
 
 
 @pytest.mark.timeout(timeout=3*WEB_TEST_TIMEOUT)
-def test_git_mergetool(git_repo, unique_port, popen_with_terminator):
+def test_git_mergetool(git_repo, unique_port, popen_with_terminator, auth_header):
     gitmergetool.main(['config', '--enable'])
     cmd = get_output('git config --get --local mergetool.nbdime.cmd').strip()
 
@@ -642,11 +642,12 @@ def test_git_mergetool(git_repo, unique_port, popen_with_terminator):
         url_concat(url + '/api/store', {'outputfilename': 'merge-conflict.ipynb'}),
         data=json.dumps({
             'merged': nbformat.v4.new_notebook(),
-        })
+        }),
+        headers=auth_header
     )
     r.raise_for_status()
     # close it
-    r = requests.post(url + '/api/closetool', json={'exitCode': 0})
+    r = requests.post(url + '/api/closetool', json={'exitCode': 0}, headers=auth_header)
     r.raise_for_status()
     # wait for exit
     process.wait()
@@ -693,7 +694,7 @@ def test_hg_mergedriver(hg_repo, filespath, reset_log):
 
 
 @pytest.mark.timeout(timeout=3*WEB_TEST_TIMEOUT)
-def test_hg_diffweb(hg_repo, unique_port, popen_with_terminator):
+def test_hg_diffweb(hg_repo, unique_port, popen_with_terminator, auth_header):
     # enable diff/merge drivers
     write_local_hg_config(hg_repo)
 
@@ -708,7 +709,7 @@ def test_hg_diffweb(hg_repo, unique_port, popen_with_terminator):
         r = requests.get(url + '/difftool')
         r.raise_for_status()
         # close it
-        r = requests.post(url + '/api/closetool', json={'exitCode': 0})
+        r = requests.post(url + '/api/closetool', json={'exitCode': 0}, headers=auth_header)
         r.raise_for_status()
         time.sleep(0.25)
     # wait for exit
@@ -717,7 +718,7 @@ def test_hg_diffweb(hg_repo, unique_port, popen_with_terminator):
 
 
 @pytest.mark.timeout(timeout=WEB_TEST_TIMEOUT)
-def test_hg_mergetool(hg_repo, unique_port, popen_with_terminator):
+def test_hg_mergetool(hg_repo, unique_port, popen_with_terminator, auth_header):
     # enable diff/merge drivers
     write_local_hg_config(hg_repo)
 
@@ -738,11 +739,12 @@ def test_hg_mergetool(hg_repo, unique_port, popen_with_terminator):
         url_concat(url + '/api/store', {'outputfilename': 'merge-conflict.ipynb'}),
         data=json.dumps({
             'merged': nbformat.v4.new_notebook(),
-        })
+        }),
+        headers=auth_header
     )
     r.raise_for_status()
     # close it
-    r = requests.post(url + '/api/closetool', json={'exitCode': 0})
+    r = requests.post(url + '/api/closetool', json={'exitCode': 0}, headers=auth_header)
     r.raise_for_status()
     # wait for exit
     process.wait()

nbdime/tests/test_web.py

@@ -24,6 +24,7 @@
 merge_a = 'multilevel-test-base.ipynb'
 merge_b = 'multilevel-test-local.ipynb'
 merge_c = 'multilevel-test-remote.ipynb'
+
 
 
 @pytest.mark.timeout(timeout=WEB_TEST_TIMEOUT)
@@ -83,12 +84,12 @@ def test_fetch_diff(http_client, base_url, nbdime_base_url):
 
 @pytest.mark.timeout(timeout=WEB_TEST_TIMEOUT)
 @pytest.mark.gen_test
-def test_api_diff(http_client, base_url, nbdime_base_url, diff_validator, filespath):
+def test_api_diff(http_client, base_url, nbdime_base_url, diff_validator, filespath, auth_header):
     post_data = dict(base=diff_a, remote=diff_b)
     body = json_encode(post_data)
 
     url = base_url + nbdime_base_url + '/api/diff'
-    response = yield http_client.fetch(url, method='POST', headers=None, body=body)
+    response = yield http_client.fetch(url, method='POST', headers=auth_header, body=body)
     assert response.code == 200
     # Check that response is sane:
     data = json_decode(response.body)
@@ -111,12 +112,12 @@ def test_fetch_merge(http_client, base_url, nbdime_base_url):
 
 @pytest.mark.timeout(timeout=WEB_TEST_TIMEOUT)
 @pytest.mark.gen_test
-def test_api_merge(http_client, base_url, nbdime_base_url, merge_validator, filespath):
+def test_api_merge(http_client, base_url, nbdime_base_url, merge_validator, filespath, auth_header):
     post_data = dict(base=merge_a, local=merge_b, remote=merge_c)
     body = json_encode(post_data)
 
     url = base_url + nbdime_base_url + '/api/merge'
-    response = yield http_client.fetch(url, method='POST', headers=None, body=body)
+    response = yield http_client.fetch(url, method='POST', headers=auth_header, body=body)
     assert response.code == 200
     # Check that response is sane:
     data = json_decode(response.body)

nbdime/webapp/nb_server_extension.py

@@ -10,27 +10,9 @@
 
 from jupyter_server.utils import url_path_join, to_os_path, ensure_async
 
-generic_checkpoint_mixin_types = []
-file_checkpoint_mixin_types = []
 
-try:
-    from jupyter_server.services.contents.checkpoints import GenericCheckpointsMixin as jpserver_GenericCheckpointsMixin
-    from jupyter_server.services.contents.filecheckpoints import FileCheckpoints as jpserver_FileCheckpoints
-    generic_checkpoint_mixin_types.append(jpserver_GenericCheckpointsMixin)
-    file_checkpoint_mixin_types.append(jpserver_FileCheckpoints)
-except ModuleNotFoundError:
-    pass
-
-try:
-    from notebook.services.contents.checkpoints import GenericCheckpointsMixin as nbserver_GenericCheckpointsMixin
-    from notebook.services.contents.filecheckpoints import FileCheckpoints as nbserver_FileCheckpoints
-    generic_checkpoint_mixin_types.append(nbserver_GenericCheckpointsMixin)
-    file_checkpoint_mixin_types.append(nbserver_FileCheckpoints)
-except ModuleNotFoundError:
-    pass
-
-generic_checkpoint_mixin_types = tuple(generic_checkpoint_mixin_types)
-file_checkpoint_mixin_types = tuple(file_checkpoint_mixin_types)
+from jupyter_server.services.contents.checkpoints import GenericCheckpointsMixin
+from jupyter_server.services.contents.filecheckpoints import FileCheckpoints
 
 
 from tornado.web import HTTPError, escape, authenticated, gen
@@ -169,11 +151,11 @@ async def _get_checkpoint_notebooks(self, base):
             return remote_nb, remote_nb
         self.log.debug('Checkpoints: %r', checkpoints)
         checkpoint = checkpoints[0]
-        if isinstance(cm.checkpoints, generic_checkpoint_mixin_types):
+        if isinstance(cm.checkpoints, GenericCheckpointsMixin):
             checkpoint_model = await ensure_async(
                 cm.checkpoints.get_notebook_checkpoint(checkpoint, base))
             base_nb = checkpoint_model['content']
-        elif isinstance(cm.checkpoints, file_checkpoint_mixin_types):
+        elif isinstance(cm.checkpoints, FileCheckpoints):
             path = await ensure_async(
                 cm.checkpoints.checkpoint_path(checkpoint['id'], base))
             base_nb = read_notebook(path, on_null='minimal')

nbdime/webapp/nbdimeserver.py

@@ -1,9 +1,7 @@
 #!/usr/bin/env python
 # -*- coding:utf-8 -*-
 
-
-
-
+import base64
 import io
 import json
 import logging
@@ -386,6 +384,7 @@ def make_app(**params):
         'jinja2_env': env,
         'mathjax_url': prefix + '/nb-static/mathjax/MathJax.js',
         'local_hostnames': ['localhost', '127.0.0.1'],
+        'cookie_secret': base64.encodebytes(os.urandom(32)), # Needed even for an unsecured server.
     }
 
     if is_in_repo(nbdime_root):

packages/labextension/package.json

@@ -62,7 +62,7 @@
     "@lumino/commands": "^1.6.1",
     "mkdirp": "^0.5.1",
     "rimraf": "^2.6.3",
-    "typescript": "^3.7.2"
+    "typescript": "^4.9.0"
   },
   "jupyterlab": {
     "extension": true,

packages/labextension/src/actions.ts

@@ -4,15 +4,15 @@ import {
   PathExt, URLExt
 } from '@jupyterlab/coreutils';
 
-import {
+import type {
   IRenderMimeRegistry
 } from '@jupyterlab/rendermime';
 
 import {
   ServerConnection
 } from '@jupyterlab/services';
 
-import {
+import type {
   Widget
 } from '@lumino/widgets';
 

packages/labextension/src/plugin.ts

@@ -2,7 +2,7 @@
 // Distributed under the terms of the Modified BSD License.
 
 
-import {
+import type {
   JupyterFrontEndPlugin, JupyterFrontEnd
 } from '@jupyterlab/application';
 
@@ -14,15 +14,15 @@ import {
   PathExt
 } from '@jupyterlab/coreutils';
 
-import {
+import type {
   DocumentRegistry
 } from '@jupyterlab/docregistry';
 
 import {
   IRenderMimeRegistry
 } from '@jupyterlab/rendermime';
 
-import {
+import type {
   INotebookModel
 } from '@jupyterlab/notebook';
 
@@ -38,7 +38,7 @@ import {
   find
 } from '@lumino/algorithm';
 
-import {
+import type {
   CommandRegistry
 } from '@lumino/commands';
 
@@ -47,7 +47,7 @@ import {
 } from '@lumino/disposable';
 
 import {
-  diffNotebookGit, diffNotebook, diffNotebookCheckpoint, isNbInGit
+  diffNotebookGit, diffNotebookCheckpoint, isNbInGit
 } from './actions';
 
 
@@ -141,13 +141,6 @@ function addCommands(
   // Whether we have our server extension available
   let hasAPI = true;
 
-  /**
-   * Whether there is an active notebook.
-   */
-  function hasWidget(): boolean {
-    return tracker.currentWidget !== null;
-  }
-
   /**
    * Whether there is an active notebook.
    */

packages/labextension/src/widget.ts

@@ -1,28 +1,28 @@
 
 
-import * as nbformat from '@jupyterlab/nbformat';
+import type * as nbformat from '@jupyterlab/nbformat';
 
-import {
+import type {
   IRenderMimeRegistry
 } from '@jupyterlab/rendermime';
 
 import {
   ServerConnection
 } from '@jupyterlab/services';
 
-import {
+import type {
   JSONObject
 } from '@lumino/coreutils';
 
-import {
+import type {
   Message
 } from '@lumino/messaging';
 
 import {
   Widget, Panel
 } from '@lumino/widgets';
 
-import {
+import type {
   IDiffEntry
 } from 'nbdime/lib/diff/diffentries';
 
@@ -144,12 +144,12 @@ class NbdimeWidget extends Panel {
     return work;
   }
 
-  protected onError(error: ServerConnection.NetworkError | ServerConnection.ResponseError): void {
+  protected onError(error: string): void {
     if (this.isDisposed) {
       return;
     }
     let widget = new Widget();
-    widget.node.innerHTML = `Failed to fetch diff: ${error.message}`;
+    widget.node.innerHTML = `Failed to fetch diff: ${error}`;
     this.scroller.addWidget(widget);
   }