Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Target Typescript compilation to es2015 #4462

Merged
merged 3 commits into from May 2, 2018
Merged

Conversation

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Apr 25, 2018

Fixes #4461.

@jasongrout jasongrout added this to the Beta 3 milestone Apr 25, 2018
@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Apr 25, 2018

@ian-r-rose - I'm running into errors with, for example, this line:

return extender[label];

How can I index into the extender (which is an IMenuExtender), and why does that give me a string?

Edit: never mind, dumb question. We are doing something a bit 'dangerous', in that we are indexing into an arbitrary object, so we aren't guaranteed to get a string back...

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented Apr 26, 2018

I think we should be able to specify the type more strictly with the new conditional mapped types of Typescript 2.8. Here
is an example in the TS playground.

I actually think there are some bugs with the implementation in TS 2.8.3 (shown in the example), but they are possible to work around.

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Apr 26, 2018

Looks like there is one error where webpack miscompiles an arrow function that I worked around. Everything I tried seems to work okay now.

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Apr 26, 2018

Upgrading webpack and typescript should be different issues.

@jasongrout jasongrout changed the title WIP Target Typescript compilation to es2015 Target Typescript compilation to es2015 Apr 26, 2018
@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Apr 26, 2018

Looks like typedoc needs to be upgraded too. Typedoc 0.9.0 uses ts 2.4, 0.10 uses ts 2.7 (which doesn't compile our code just yet). Investigating...

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Apr 26, 2018

I took out the small enhancements we made to the main menu find function at put them at #4468, which was causing the problems with older versions of typescript that typedoc was using.

Now the changes in this PR are only about the es2015 target, and hopefully are simpler and pass the tests.

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Apr 26, 2018

Tests look like they are passing now.

On the larger discussion about this change, this requires a major version bump for everything, including services, coreutils, rendermime-interfaces, and observables, which have already reached 1.0 or later.

What it gives us is proper iterator support (for iterating es2015 data structures, for example), that we didn't have available before (for example for of iteration of a set).

What it gives us in the generated code is much better code for async/await (good enough that I think we can start using async/await where ever we want in the codebase without making debugging too difficult), and code in the browsers that uses es2015 features such as arrow functions, es6 classes, let, const, and many other things.

What we give up is IE 11 support, especially for libraries like services that previously could be used in IE 11. (This will affect ipywidgets, but I don't think it will be a problem in the short term since we can use services 2.0 in cases where we officially support IE 11).

@ellisonbg, @sccolbert, and anyone else - Thoughts?

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Apr 26, 2018

From @Carreau - stats on jupyter.org show that IE+Edge = 6% and IE 11 itself little under 3% 'market share'

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Apr 26, 2018

That mirrors what many other sites show - IE 11 is low single digit market share. Wikipedia has the highest that I saw, with about 7% market share.

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Apr 27, 2018

Looks like my worries were overblown. Jupyter notebook (even version 4) officially does not support IE 11 (partly because of its buggy flexbox implementation). We do not need to worry about IE 11 support for widgets either, then. So definitely +1 on migrating to es2015 output.

Copy link
Member

@ian-r-rose ian-r-rose left a comment

Thanks for pushing forwards on this @jasongrout

@ian-r-rose ian-r-rose merged commit 8428606 into jupyterlab:master May 2, 2018
2 checks passed
@saulshanabrook
Copy link
Member

@saulshanabrook saulshanabrook commented May 8, 2018

Looks like typedoc needs to be upgraded too. Typedoc 0.9.0 uses ts 2.4, 0.10 uses ts 2.7 (which doesn't compile our code just yet). Investigating...

How did you work around this? I can't build the docs in #4512 because it won't handle some JSX things, because the typescript is too old. However, there doesn't seem to be a version of typedoc that support TS 2.6 (what we are using). If I upgrade to that version of typedoc, then it can't compile some other code.

Is the best route to upgrade the whole codebase to 2.7 and upgrade typedoc?

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented May 8, 2018

How did you work around this?

I didn't. I left the ts version the same.

Is the best route to upgrade the whole codebase to 2.7 and upgrade typedoc?

I think so.

@ian-r-rose ian-r-rose mentioned this pull request Jul 20, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Aug 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants