Convert frontend to typescript #314

ianhi · 2021-03-24T05:41:29Z

Switch the frontend code from js to ts
Fix several small errors that typescript detected
Add proper labextension:watch script to package.json
Generally update the structure of the frontend to better track https://github.com/jupyter-widgets/widget-ts-cookiecutter for easier future maintainability.

I think the arguments for switching to typescript are:

Most of the jupyterlab ecosystem has a preference for typescript (including widgets)
Can catch errors (found some in this PR)
Can be more accessible to potential contributors coming from a pure python background. (anecdotal of course but this was very much true for me)

@martinRenou what do you think?

github-actions · 2021-03-24T05:41:43Z

👈 Launch a binder notebook on branch ianhi/ipympl/ts

martinRenou · 2021-03-24T08:13:38Z

That's amazing. Thanks for doing this!

ianhi · 2021-04-07T03:49:57Z

I've been stuck on this:

In a fresh environment if I follow the steps as in the github action then that directory exists for me - so I'm not really sure how to fix. Sometime I may have a go at just starting from the cookiecutter more fully and see if that helps.

martinRenou · 2021-04-07T06:40:20Z

Sometime I may have a go at just starting from the cookiecutter more fully and see if that helps

Yeah. I was wondering if it was not best to do that indeed. I might have some time allocated for working on ipympl soonish, so if you don't have time and if you want I might be able to follow up on this.

martinRenou · 2021-04-07T07:27:35Z

Although I believe your approach should work as well. I am not sure what the nbextension issue is. It might be just that the typescript compilation failed but the error didn't propagate and pip install . succeeded wrongly resulting in a broken installation. I've seen this happening once.

I can have a try.

ianhi · 2021-04-07T17:50:26Z

Sometime I may have a go at just starting from the cookiecutter more fully and see if that helps
Yeah. I was wondering if it was not best to do that indeed.

My worry was that doing this may make it would make it much harder to maintain the git history

It might be just that the typescript compilation failed but the error didn't propagate and pip install . succeeded wrongly resulting in a broken installation. I've seen this happening once.

Interesting, looking through the logs that doesn't seem to have happened, but I'll try to look more closely sometime.

I might have some time allocated for working on ipympl soonish, so if you don't have time and if you want I might be able to follow up on this.

Cool, I'll keep trying of course, but not guarantees w.r.t success.

ianhi · 2021-04-07T18:33:29Z

I think part of the issue was that I had accidentally deleted the ipympl/nbextension/extension.js

ianhi · 2021-04-07T18:35:24Z

docs/source/_static/embed-bundle.js

@@ -0,0 +1,810 @@
+define("jupyter-matplotlib", ["@jupyter-widgets/base"], (__WEBPACK_EXTERNAL_MODULE__jupyter_widgets_base__) => { return /******/ (() => { // webpackBootstrap


woops this file snuck in at some points. Will need to remove this in a rebase at some point prior to merging

ianhi · 2021-04-07T18:45:39Z

Getting somewhere!

ianhi · 2021-04-07T19:00:42Z

The LGTM for javascript should either be disabled or swapped to typescript. I'm not sure who has the rights to do that, but probably @tacaswell ?

SylvainCorlay · 2021-04-07T19:03:31Z

Thank you so much @ianhi !

ianhi · 2021-04-07T19:07:57Z

hooorayy the tests all pass!

There are a few things I still want to clean up and #317 should probably be merged first so let's not merge just yet.

TODO:

Remove the embed-bundle under docs
Update readme as necessary
Squash some of the commits

ianhi · 2021-06-04T18:57:53Z

test failures are due to the churn in jupyter_packaging which I believe is settled now:

ImportError: cannot import name 'get_version_info' from 'jupyter_packaging'

I think it's best to also convert to the method described here: https://github.com/jupyter/jupyter-packaging#as-a-build-backend

Though I admit how that works is very mysterious for me so if anyone feels inspired to have a crack at it please feel free to push here.

martinRenou · 2021-09-21T14:34:03Z

Sorry for not looking into this earlier. I will rebase and fix the conflicts.

It works great in JupyterLab but I have an issue in Jupyter Notebook, I'll see if I can figure it out.

In the process also update some the tooling (eslint, prettier, setup.py) to more closely the widget-ts-cookiecutter. Also fix miscellanous errors discovered by using typescript. Finally, add a labextension watch command. lint the correct directory Update MANIFEST.in for TS correctly install labextension in the test Return to using yarn in setup.pt

martinRenou · 2021-09-21T14:43:55Z

Rebased, it seems to work now in Jupyter Notebook as well.

Let's see what CI thinks about it.

martinRenou · 2021-09-21T15:02:02Z

Let's goooo! Thank you a lot for this!

ianhi · 2021-09-21T15:37:53Z

hooorayy! thanks for pushing it over the edge. I got it working in lab but never could figure out the notebook

ianhi · 2021-09-21T15:38:24Z

ipympl/nbextension/extension.js

@@ -6,7 +6,7 @@ define(function() {
    window['requirejs'].config({
        map: {
            '*': {
-                'jupyter-matplotlib': 'nbextensions/ipympl/index',


wow that was it??? nice find

ianhi commented Apr 7, 2021

View reviewed changes

ianhi mentioned this pull request Apr 7, 2021

Handle new FigureCanvasBase.device_pixel_ratio. #317

Merged

ianhi force-pushed the ts branch 2 times, most recently from 39f6c4b to 2b1b389 Compare April 8, 2021 03:39

ianhi force-pushed the ts branch from 2b1b389 to 78a13b9 Compare April 16, 2021 21:37

ianhi and others added 9 commits September 21, 2021 16:40

rename to .ts

f177df3

move js-> src

dc59c1f

extend gitignore for typescript

fb2605c

clean up issues with gitignore

b541997

Use built wheel for the tests

c17a0dc

run prettier

522f5e8

Update readme for typescript

1c582de

Fix Jupyter Notebook

8e0538b

martinRenou force-pushed the ts branch from 83e4a3f to 8e0538b Compare September 21, 2021 14:43

README: Add --overwrite option to the nbextension install

ae0ec2d

martinRenou merged commit 4f390a0 into matplotlib:master Sep 21, 2021

ianhi deleted the ts branch September 21, 2021 15:37

ianhi commented Sep 21, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert frontend to typescript #314

Convert frontend to typescript #314

ianhi commented Mar 24, 2021

github-actions bot commented Mar 24, 2021

martinRenou commented Mar 24, 2021

ianhi commented Apr 7, 2021

martinRenou commented Apr 7, 2021

martinRenou commented Apr 7, 2021

ianhi commented Apr 7, 2021

ianhi commented Apr 7, 2021

ianhi Apr 7, 2021

ianhi commented Apr 7, 2021

ianhi commented Apr 7, 2021

SylvainCorlay commented Apr 7, 2021

ianhi commented Apr 7, 2021

ianhi commented Jun 4, 2021

martinRenou commented Sep 21, 2021

martinRenou commented Sep 21, 2021

martinRenou commented Sep 21, 2021

ianhi commented Sep 21, 2021

ianhi Sep 21, 2021

		@@ -0,0 +1,810 @@
		define("jupyter-matplotlib", ["@jupyter-widgets/base"], (__WEBPACK_EXTERNAL_MODULE__jupyter_widgets_base__) => { return /******/ (() => { // webpackBootstrap

Convert frontend to typescript #314

Convert frontend to typescript #314

Conversation

ianhi commented Mar 24, 2021

github-actions bot commented Mar 24, 2021

martinRenou commented Mar 24, 2021

ianhi commented Apr 7, 2021

martinRenou commented Apr 7, 2021

martinRenou commented Apr 7, 2021

ianhi commented Apr 7, 2021

ianhi commented Apr 7, 2021

ianhi Apr 7, 2021

Choose a reason for hiding this comment

ianhi commented Apr 7, 2021

ianhi commented Apr 7, 2021

SylvainCorlay commented Apr 7, 2021

ianhi commented Apr 7, 2021

ianhi commented Jun 4, 2021

martinRenou commented Sep 21, 2021

martinRenou commented Sep 21, 2021

martinRenou commented Sep 21, 2021

ianhi commented Sep 21, 2021

ianhi Sep 21, 2021

Choose a reason for hiding this comment