Upgrade to Typescript 3.7 RC #7428

jasongrout · 2019-10-28T16:47:37Z

References

Code changes

This is a test PR to see how easy it will be to upgrade to Typescript 3.7 in the coming weeks.

User-facing changes

None

Backwards-incompatible changes

The Typescript definition files may be backwards incompatible. We remove the vega4 extension and upgrade the vega5 extension to vega-lite 4.0 beta, which supports TS 3.7.

jupyterlab-dev-mode · 2019-10-28T16:47:45Z

Thanks for making a pull request to JupyterLab!

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

jasongrout · 2019-10-28T16:49:31Z

I get one error doing jlpm run build with this:

We'll pick up the log after the vega packages built, when it is building the meta package:

$ cd packages/metapackage && jlpm run build
$ tsc -b
../../node_modules/vega-lite/build/src/compile/selection/selection.d.ts:11:10 - error TS2440: Import declaration conflicts with local declaration of 'SelectionComponent'.

11 import { SelectionComponent } from './selection';
            ~~~~~~~~~~~~~~~~~~

../../node_modules/vega-lite/build/src/compile/selection/selection.d.ts:11:10 - error TS2440: Import declaration conflicts with local declaration of 'SelectionComponent'.

11 import { SelectionComponent } from './selection';
            ~~~~~~~~~~~~~~~~~~


Found 2 errors.

error Command failed with exit code 1.

jasongrout · 2019-10-28T16:53:49Z

Indeed, here are the first few lines of that file:

import { Channel, ScaleChannel } from '../../channel';
import { LogicalOperand } from '../../logical';
import { BrushConfig, SelectionDef, SelectionResolution, SelectionType } from '../../selection';
import { Dict } from '../../util';
import { VgBinding, VgData, VgEventStream, VgSignalRef } from '../../vega.schema';
import { DataFlowNode } from '../data/dataflow';
import { TimeUnitNode } from '../data/timeunit';
import { LayerModel } from '../layer';
import { Model } from '../model';
import { UnitModel } from '../unit';
import { SelectionComponent } from './selection';
export declare const STORE = "_store";
export declare const TUPLE = "_tuple";
export declare const MODIFY = "_modify";
export declare const SELECTION_DOMAIN = "_selection_domain_";
export interface SelectionComponent {
    name: string;
    type: SelectionType;
    events: VgEventStream;
    bind?: 'scales' | VgBinding | {
        [key: string]: VgBinding;
    };
    resolve: SelectionResolution;
    empty: 'all' | 'none';
    mark?: BrushConfig;
    _signalNames: {};
    project?: ProjectComponent[];
    fields?: any;
    timeUnit?: TimeUnitNode;
    scales?: Channel[];
    toggle?: any;
    translate?: any;
    zoom?: any;
    nearest?: any;
}

Looking at node_modules/vega-lite/package.json, I see that this is vega-lite 2.7.0.

@domoritz - what do you think? Several options come to mind:

Drop vega4 support in jlab 2.0 (due in January 2020) (I think that's why we are pulling in vega-lite 2.7.0?)
Fix vega-lite 2.7.0 or our build of it
Perhaps this is a Typescript issue? If so, report it upstream?

domoritz · 2019-10-28T17:12:38Z

It’s a bug in Vega-Lite 2.7 but we usually do not backport minor fixes. I’d say drop 2.7 from JL 2.

jasongrout · 2019-10-28T17:46:19Z

Will vega-lite 4 be ready by January? Can we move our support to vega-lite 3 and 4?

domoritz · 2019-10-28T18:05:36Z

Yes, our plan is to release in November.

jasongrout · 2019-10-28T19:07:54Z

Okay, thanks. I'll let you/others update jlab 2.0's vega roadmap (whether we just update the existing vega5 extension to vega-lite 4, or whether we need to make a new extension). I'll remove the vega4 extension here.

jasongrout · 2019-10-28T19:12:33Z

It’s a bug in Vega-Lite 2.7 but we usually do not backport minor fixes. I’d say drop 2.7 from JL 2.

Just curious, would you say that vega-lite 2 is unmaintained at this point?

domoritz · 2019-10-28T19:41:22Z

From version to version, we only break very few things and so I'd say that if there is a bug, you can almost always upgrade without any changes to your spec.

See discussion on jupyterlab#7428 for more information.

jasongrout · 2019-10-28T19:47:41Z

Okay, thanks.

I pushed some commits that remove the vega 4 extension and upgrade the vega 5 extension to the latest vega and vega-lite. Now I'm seeing these errors when building JupyterLab, still from our builds of vega

$ cd packages/metapackage && jlpm run build
$ tsc -b
../../node_modules/vega-lite/build/src/transform.d.ts:10:10 - error TS2440: Import declaration conflicts with local declaration of 'JoinAggregateTransform'.

10 import { JoinAggregateTransform } from './transform';
            ~~~~~~~~~~~~~~~~~~~~~~

../../node_modules/vega-lite/build/src/projection.d.ts:1:10 - error TS2440: Import declaration conflicts with local declaration of 'ProjectionType'.

1 import { ProjectionType } from './vega.schema';
           ~~~~~~~~~~~~~~

../../node_modules/vega-lite/build/src/spec/repeat.d.ts:4:10 - error TS2440: Import declaration conflicts with local declaration of 'RepeatMapping'.

4 import { RepeatMapping } from './repeat';
           ~~~~~~~~~~~~~

../../node_modules/vega-lite/build/src/spec/facet.d.ts:7:10 - error TS2440: Import declaration conflicts with local declaration of 'FacetMapping'.

7 import { FacetMapping } from './facet';
           ~~~~~~~~~~~~

../../node_modules/vega-lite/build/src/bin.d.ts:1:10 - error TS2440: Import declaration conflicts with local declaration of 'BinParams'.

1 import { BinParams } from './bin';
           ~~~~~~~~~

../../node_modules/vega-lite/build/src/legend.d.ts:5:71 - error TS2315: Type 'LegendConfig' is not generic.

5 export declare type LegendConfig = LegendMixins & VlOnlyGuideConfig & VgLegendConfig<number, number, string, Color, FontWeight, FontStyle, Align, TextBaseline, LayoutAlign, LabelOverlap, SymbolShape, number[], Orient, TitleAnchor, LegendOrient> & {
                                                                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

../../node_modules/vega-lite/build/src/legend.d.ts:46:33 - error TS2315: Type 'BaseLegend' is not generic.

46 export interface Legend extends BaseLegend<number, number, string, Color, FontWeight, FontStyle, Align, TextBaseline, LayoutAlign, LabelOverlap, SymbolShape, number[], Orient, TitleAnchor, LegendOrient>, LegendMixins, Guide {
                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

../../node_modules/vega-lite/build/src/axis.d.ts:5:47 - error TS2315: Type 'BaseAxis' is not generic.

5 declare type BaseAxisNoSignals = AxisMixins & BaseAxis<number, number, boolean, number | boolean, string, Color, FontWeight, FontStyle, Align, TextBaseline, LayoutAlign, LabelOverlap, number[], TitleAnchor>;
                                                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

../../node_modules/vega-lite/build/src/axis.d.ts:119:63 - error TS2677: A type predicate's type must be assignable to its parameter's type.
  Type 'string | number' is not assignable to type 'string'.
    Type 'number' is not assignable to type 'string'.

119 export declare function isAxisProperty(prop: string): prop is keyof Axis;
                                                                  ~~~~~~~~~~


Found 9 errors.

domoritz · 2019-10-28T20:08:08Z

I'll update Vega-Lite to Typescript 3.7 to see whether it's a bug on our side.

domoritz · 2019-10-28T22:19:59Z

I fixed the problems we had in our codebase. They weren't really wrong since we just imported a type that was defined in the same file but I can see why we should avoid this.

saulshanabrook · 2019-11-10T12:45:30Z

I pushed some commits that remove the vega 4 extension and upgrade the vega 5 extension to the latest vega and vega-lite.

@jasongrout just a reminder that if we remove the vega 4 extension we can get rid of our pre-builds of the vega package since we only have one version to deal with, if we want.

jasongrout · 2019-11-11T19:53:40Z

Thanks.

As an update, Typescript 3.7 final is out now.

jasongrout · 2019-11-11T19:57:20Z

Also, a vega-lite beta was published (https://github.com/vega/vega-lite/releases/tag/v4.0.0-beta.11), so we can update to that and continue working on this.

jasongrout · 2019-11-11T20:46:46Z

This appears to build and run now. Next up is removing the pre-build of the vega stuff.

jasongrout · 2019-11-11T21:14:18Z

@domoritz - just to clarify, we're updating to vega-lite 4 in this PR. Can you see a reason why we'd need to ship vega-lite 3 as well in jlab 2?

c.f. the comment from Saul that we can remove this now that we don’t have multiple versions in core: jupyterlab#7428

jasongrout · 2019-11-12T01:32:14Z

Is there an easy way to fall back to a newer Vega-Lite mimetype?

I think the proper way to do this would be for vega-lite to ship a function to convert a vl3 spec to a vl4 spec. 99% of the time it does nothing but change the schema line, apparently. That way people can read vl3 files with the latest version of vega-lite in an officially supported way, rather than a "let's see if it works, maybe" way.

jasongrout · 2019-11-12T01:34:06Z

I think the proper way to do this would be for vega-lite to ship a function to convert a vl3 spec to a vl4 spec.

Even better would be for the generic vl4 rendering method to silently convert to vl4 and render, so vl4 is able to render older versions of the spec transparently. That would make our lives a lot easier (we don't have to worry about versions, we just hand the data to the latest vega-lite), and would be an official supported thing from the vega project, rather than us just guessing that maybe it would work.

domoritz · 2019-11-12T01:48:06Z

Even better would be for the generic vl4 rendering method to silently convert to vl4 and render, so vl4 is able to render older versions of the spec transparently. That would make our lives a lot easier (we don't have to worry about versions, we just hand the data to the latest vega-lite), and would be an official supported thing from the vega project, rather than us just guessing that maybe it would work.

We already do this for cases where it makes sense. In rare cases, we don't do this for example in Vega-Lite 4 where we are removing support for rangeStep. I don't think we can promise backward compatibility for all future versions but we are making an effort to stay compatible.

jasongrout · 2019-11-12T02:16:11Z

Can you have a function that throws an error or something when it can't convert? Then we could throw the data into your function, and display a warning if it doesn't work.

In other words, we really want something that is officially supported by the library (possibly with errors when it can't do something), rather than just trying a fallback on our own.

domoritz · 2019-11-12T02:24:48Z

We can add that. Can you make sure that Altair can use the latest renderer available and that users can install an extension for a specific version if there is an error that they cannot fix?

jasongrout · 2019-11-12T02:59:30Z

Can you make sure that Altair can use the latest renderer available

We can open an issue for the Altair project. I'm thinking that on the jlab side, if we find any of the vega-lite mimetypes coming through, we just ask our vega-lite extension to render it, and if it throws an error, we display a message to the user saying that they will need to install an extension to render it.

domoritz · 2019-11-12T03:07:40Z

I think Altair always generates a specific mimetype since jlab does not support multiple mimetype for the same document/file (not sure what the terminology is). We will update Vega-Lite to show warnings for unsupported properties. Vega-Lite never throws errors when it can just ignore properties.

domoritz · 2019-11-12T03:14:54Z

I filed vega/vega#2149 and vega/vega-lite#5538 to track the changes on the Vega and Vega-Lite side.

jasongrout · 2019-11-13T14:23:22Z

@domoritz - just curious, are you also planning to ship a vl 3 patch release that supports TS 3.7?

jasongrout · 2019-11-13T16:49:42Z

I think Altair always generates a specific mimetype since jlab does not support multiple mistypes for the same document/file (not sure what the terminology is).

Just to follow up on this - my understanding is that the vega-lite format has the version number embedded in it as the $schema key, so generating a single plot that targets multiple versions of vega-lite isn't possible, right?

domoritz · 2019-11-13T18:35:49Z

Just to follow up on this - my understanding is that the vega-lite format has the version number embedded in it as the $schema key, so generating a single plot that targets multiple versions of vega-lite isn't possible, right?

The $schema is optional and Vega/Vega-Lite do not even look at the version when parsing a spec. The main issue (as I see it) is that the Vega mimetype has the version baked into it (https://github.com/jupyterlab/jupyterlab/blob/master/packages/vega5-extension/src/index.ts#L35).

jasongrout · 2019-11-13T18:53:16Z

The $schema is optional and Vega/Vega-Lite do not even look at the version when parsing a spec.

Ah, I didn't realize it was optional.

I think you need a version somewhere, otherwise you don't know how to communicate exactly what dialect/version you are speaking. I think what you are saying is that it would be useful to say "this plot compatible with version 3 and version 4"? Who would keep track of that? Altair?

domoritz · 2019-11-13T22:33:08Z

@jasongrout Thanks for taking the time to talk today. I chatted with @kanitw and Vega-Lite actually maintains backward compatibility to v3 by converting v4 to v3 specs. So let's continue with the plan as we discussed today. You can just use v4 for both VL3 and VL4 specs.

jasongrout · 2019-11-14T14:09:59Z

@domoritz - trying out the current code of this PR (which just calls the current 4.0b11 renderer with whatever v3 document altair gives us) prints out "The input spec uses vega-lite v3.4.0, but the current version of Vega-Lite is 4.0.0-beta.11" to the console as a warning, and then displays some errors rather than graphs, like:

Javascript Error: Unrecognized transform type: "filter" for https://altair-viz.github.io/gallery/simple_heatmap.html or https://altair-viz.github.io/gallery/simple_bar_chart.html, or Javascript Error: Unrecognized transform type: "formula" for https://altair-viz.github.io/gallery/simple_line_chart.html

jasongrout · 2019-11-14T14:12:29Z

(Again, happy to take this up on another issue in a few weeks when 4.0 ships, if you can release a v3 that supports ts 3.7 :)

jasongrout · 2019-11-14T14:45:18Z

then displays some errors rather than graphs, like:

Ah, looks like the problem went away when I updated vega-embed and vega to the most recent versions. I can now render lots of altair examples.

I still see the warning message about rendering a vega-lite 3 spec in the console, though. Perhaps that can go away after implementing what we talked about, about being able to explicitly support rendering vl3 documents?

jasongrout · 2019-11-14T15:12:03Z

I did some more testing, and it looks like most of the altair examples work after upgrading vega-embed and vega. However, I'm still seeing issues with charts that involve maps that don't render. For example, https://altair-viz.github.io/gallery/airports_count.html gives

with a lot of js errors like:

ERROR TypeError: "undefined has no properties"
    shape shapes.js:73
    bound markItemPath.js:20
    boundItem Bound.js:80
    transform Bound.js:41
    transform Bound.js:40
    evaluate Transform.js:55
    run Transform.js:33
    evaluate run.js:74
logger.js:3
    log logger.js:3
    error logger.js:24
    logMethod Dataflow.js:114
    evaluate run.js:109
ERROR TypeError: "scene is undefined"
    draw CanvasRenderer.js:113
    draw group.js:91
    visit visit.js:35
    draw group.js:90
    visit visit.js:35
    draw group.js:56
    draw CanvasRenderer.js:115
    _render CanvasRenderer.js:103
    _call Renderer.js:102
    render Renderer.js:105
    renderAsync Renderer.js:132
    evaluate View.js:122
logger.js:3
    log logger.js:3
    error logger.js:24
    logMethod Dataflow.js:114
    evaluate View.js:126

Typscript 3.7 throws some errors when compiling Vega-lite 2 and 3, so we turn off lib checks. This was investigated much more deeply in jupyterlab#7428, where we deleted the vega 4 extension and upgraded the vega 5 extension to vega-lite 4.0b11. The vega-lite upgrade was not going smoothly, so we moved it to a separate issue and redid the Typescript upgrade by just using the skipLibCheck flag.

jasongrout · 2019-11-14T16:22:13Z

I've opened #7522 (Typescript upgrade) and #7523 (issue for vega-lite upgrade) to supersede this PR. I think we do the vega upgrade in a separate PR after vega-lite 4.0 comes out (or perhaps has an RC or at least is closer to a final release). That would cleanly separate the vega-lite upgrade issues from this Typescript upgrade issue.

Upgrade to Typescript 3.7 RC

d502c3d

jasongrout added this to the 2.0 milestone Oct 28, 2019

jasongrout added 3 commits October 28, 2019 12:45

Remove vega 4 extension

4d770b0

See discussion on jupyterlab#7428 for more information.

Upgrade vega-lite 3 and vega 5 dependencies

0230cb9

Update yarn.lock to reflect the vega upgrades and vega 4 removal.

f6d2feb

domoritz mentioned this pull request Oct 28, 2019

fix: ensure compatibility with TS 3.7 vega/vega-lite#5497

Merged

jasongrout added the status:Work in Progress label Nov 1, 2019

jasongrout and others added 4 commits November 11, 2019 12:16

Update to Typescript 3.7 final

3bc8037

Upgrade vega-lite to the beta that supports TypeScript 3.7

fabe76e

Update from build to remove unnecessary css import

262bdd8

Merge branch 'master' into ts37

d4ebaaf

jasongrout added 2 commits November 11, 2019 13:15

Remove build system for multiple versions of vega

156327a

c.f. the comment from Saul that we can remove this now that we don’t have multiple versions in core: jupyterlab#7428

Merge branch 'ts37' of github.com:jasongrout/jupyterlab into ts37

42ec12e

This was referenced Nov 12, 2019

Show warnings for removed properties vega/vega-lite#5538

Closed

Show warnings for removed properties vega/vega#2149

Open

Refined vega-lite3 vs vega-lite4 support.

70c6296

jasongrout force-pushed the ts37 branch from 775e9b3 to 70c6296 Compare November 14, 2019 06:49

Upgrade vega-embed and vega

b5d234c

This was referenced Nov 14, 2019

Upgrade to TypeScript 3.7 #7522

Merged

Remove vega 4 extension #7523

Closed

jasongrout closed this Nov 14, 2019

tgeorgeux mentioned this pull request Nov 20, 2019

Weekly Dev Call Meeting Minutes 2019-11-13 jupyterlab/frontends-team-compass#29

Closed

lock bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Dec 14, 2019

lock bot locked as resolved and limited conversation to collaborators Dec 14, 2019

Upgrade to Typescript 3.7 RC #7428

Upgrade to Typescript 3.7 RC #7428

Conversation

jasongrout commented Oct 28, 2019 • edited

References

Code changes

User-facing changes

Backwards-incompatible changes

jupyterlab-dev-mode bot commented Oct 28, 2019

jasongrout commented Oct 28, 2019

jasongrout commented Oct 28, 2019

domoritz commented Oct 28, 2019

jasongrout commented Oct 28, 2019

domoritz commented Oct 28, 2019

jasongrout commented Oct 28, 2019

jasongrout commented Oct 28, 2019

domoritz commented Oct 28, 2019 • edited

jasongrout commented Oct 28, 2019

domoritz commented Oct 28, 2019

domoritz commented Oct 28, 2019

saulshanabrook commented Nov 10, 2019

jasongrout commented Nov 11, 2019

jasongrout commented Nov 11, 2019

jasongrout commented Nov 11, 2019

jasongrout commented Nov 11, 2019

jasongrout commented Nov 12, 2019

jasongrout commented Nov 12, 2019

domoritz commented Nov 12, 2019

jasongrout commented Nov 12, 2019

domoritz commented Nov 12, 2019

jasongrout commented Nov 12, 2019

domoritz commented Nov 12, 2019 • edited

domoritz commented Nov 12, 2019

jasongrout commented Nov 13, 2019

jasongrout commented Nov 13, 2019

domoritz commented Nov 13, 2019

jasongrout commented Nov 13, 2019

domoritz commented Nov 13, 2019

jasongrout commented Nov 14, 2019 • edited

jasongrout commented Nov 14, 2019

jasongrout commented Nov 14, 2019

jasongrout commented Nov 14, 2019

jasongrout commented Nov 14, 2019

jasongrout commented Oct 28, 2019 •

edited

domoritz commented Oct 28, 2019 •

edited

domoritz commented Nov 12, 2019 •

edited

jasongrout commented Nov 14, 2019 •

edited