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

[types-2.0] Parallel Version Structure to Support D3 v4 Standard Bundle Definition and D3 v3 #11367

Closed
tomwanzek opened this issue Sep 21, 2016 · 20 comments
Labels

Comments

@tomwanzek
Copy link
Contributor

tomwanzek commented Sep 21, 2016

D3 version 4 can be loaded on a module-by-module basis, but for convenience a D3 Standard Bundle is provided for both module import and vanilla script use.

This D3 Standard bundle is published as a UMD module of name d3. Currently, the definitions for this standard bundle cannot be published in DefinitelyTyped/types-2.0 (see closed PR #10453) due to existing dependencies of other libraries on the legacy D3 v3.x definitions (e.g. plottable, nvd3) contained in the same directory.

This issue is spun off from the original Definitions Request #9936, where it has been tracked so far.

This issue is related to a functional gap in the current setup of the DefinitelyTyped/types-2.0 Branch. The need for a parallel version structure is a known open item as per the TypeScript team, see issue #10711.

This issue carries over the migration issue for the existing D3 version 4 standard bundle defintion file which was tracked under d3-v4-definitelytyped Issue 116.

@RyanCavanaugh @andy-ms @mhegazy

cc @gustavderdrache

With the sincerest hope that it remains a temporary workaround, developers who need a definition file representing the D3 version 4 may proceed as follows:

  1. Install the module-level definitions for the D3 modules comprising the D3 standard bundle individually, using e.g. npm install @types/d3-selection --save and so forth

  2. Create your own definition file for the standard bundle by re-exporting the individual module definitions and creating the d3 global, if required for a vanilla script project (see below for the file content.)

  3. Add the hand-crafted definition file to your project compilation context

  4. Use

    (a) as a module follows the standard TS module import nomenclature import * as d3 from 'd3'; (or subsets.
    (b) for a vanilla script, makes the global d3 available by including \\\ <reference types='d3' />

The content of the standard bundle definition file is as follows (export as namespace d3; exposes the global):

// d.ts file

export as namespace d3;

export * from 'd3-array';
export * from 'd3-axis';
export * from 'd3-brush';
export * from 'd3-chord';
export * from 'd3-collection';
export * from 'd3-color';
export * from 'd3-dispatch';
export * from 'd3-drag';
export * from 'd3-dsv';
export * from 'd3-ease';
export * from 'd3-force';
export * from 'd3-format';
export * from 'd3-geo';
export * from 'd3-hierarchy';
export * from 'd3-interpolate';
export * from 'd3-path';
export * from 'd3-polygon';
export * from 'd3-quadtree';
export * from 'd3-queue';
export * from 'd3-random';
export * from 'd3-request';
export * from 'd3-scale';
export * from 'd3-selection';
export * from 'd3-shape';
export * from 'd3-time';
export * from 'd3-time-format';
export * from 'd3-timer';
export * from 'd3-transition';
export * from 'd3-voronoi';
export * from 'd3-zoom';

Edit: The correct Definition Request Issue is #9936.

@tomwanzek
Copy link
Contributor Author

@RyanCavanaugh @andy-ms Is there an update on the ability to maintain current and legacy version in DT concurrrently? This feature is still causing issues, as people expect to find D3 v4 for the standard bundle in the d3 folder and npm @types/d3, but get caught out downloading the D3 v3.5.x legacy definitions.

This feature seems critical in general not just for D3, so an update/ETA would be great.

Thanks in advance.

@vvakame vvakame added the @types label Oct 16, 2016
@ghost
Copy link

ghost commented Oct 24, 2016

Since the legacy version has already been published, you could go through the packages depending on it and give them a package.json with { "dependencies": { "d3": "3.5.56" } }. Then when you publish d3 4.2 typings, these packages will continue using the old typings. The thing we have trouble supporting is adding new versions of legacy typings.

@tomwanzek
Copy link
Contributor Author

@andy-ms Thanks for the clarification. So, if I sum up the todo would be the following:

1, Find all definitions which have a dependency on legacy D3 (v3)
2. Add a package.json stub, with dependencies for each affected definition
3. Move the legacy definitions in d3 to a file d3-v3.5.d.ts and retire the old tests similarly for historic reference
4. Add the new definitions in index.d.ts and use new d3-tests.ts
5. Submit PR as usual for types-2.0. I will cc all the affected package definition authors.

I did not realize that the Definition tester running the integration tests over DT/types-2.0 respects the package.json dependencies. I.e. I guess it installs the dependencies as part of the PR checks?

Which leads me to the last question: I suppose the package.json-stub would have the dependency on "@types/d3": "3.5.56"? Do I need to include the actual d3 dependency as well?

Thanks as always for all your hard work!!! 😄

@ghost
Copy link

ghost commented Oct 24, 2016

Actually, depend on >=3.5.56. That way, if we add the functionality to publish new versions of old typings, these packages can get the updates. And yes, "d3" was a typo, you should use "@types/d3".
It's possible that definition-tester will hassle you, even if it works when you run npm install before tsc. Notify me if that happens.
The rest of the steps you listed look right. 👍

@tomwanzek
Copy link
Contributor Author

Thanks, will let you know. I will try to fit it in today, I am just not 100% how many other packages with D3 dependencies I will have to hit. Cheers, T.

@gustavderdrache
Copy link
Contributor

@tomwanzek At least for starters, you could try looking at the files changed list from #4032. Those were the *.d.ts files I had to update when changing the v3 typings.

@tomwanzek
Copy link
Contributor Author

@gustavderdrache we also have the list of failed Travis tests from the initial attempt to merge D3v4.

@andy-ms To take a concrete example nvd3. Other than adding the package.json stub, What changes should be made to the tsconfig.json? Currently:

{
    "compilerOptions": {
        "module": "commonjs",
        "target": "es6",
        "noImplicitAny": false,
        "strictNullChecks": false,
        "baseUrl": "../",
        "typeRoots": [
            "../"
        ],
        "types": [],
        "noEmit": true,
        "forceConsistentCasingInFileNames": true
    },
    "files": [
        "index.d.ts"
    ]
}

Is the idea that the nvd3 folder becomes self-contained with npm install and .gitignore for node_modules? I.e. what about baseUrl and typeRoots and types properties?

Sorry for the follow-up question...

@ghost
Copy link

ghost commented Oct 24, 2016

You don't need to make any other changes. There may still be other dependencies from baseUrl.

@tomwanzek
Copy link
Contributor Author

tomwanzek commented Oct 24, 2016

Alright, will have to touch:

  • c3
  • cal-heatmap
  • d3-tip (possibly, will double check)
  • d3.cloud.layout
  • d3Kit
  • dagre-d3
  • dc
  • nvd3
  • plottable

On my way, stay tuned 😉

@gustavderdrache
Copy link
Contributor

Best of luck, and heartfelt thanks for your continued diligence.

@tomwanzek
Copy link
Contributor Author

@gustavderdrache Quick question to you: What is the scoop about the /plugin sub-folder under d3? Any suggestions regarding its go-forward necessity for the legacy definitions?

@gustavderdrache
Copy link
Contributor

I only see one plugin in it. I think it's worth pinging the higher-ups to see if we can just move superformula to a top-level definition and remove that directory.

@tomwanzek
Copy link
Contributor Author

@andy-ms I have pushed a commit to my fork of DefinitelyTyped, branch replace-d3-legacy

See here for the commit

Before creating a PR, this is where it stands:

Essentially, all it does is add the package.json stub mentioned above and the changes in d3 itself. Without any changes to the relevant tsconfig.json, the individual definitions depending on legacy D3 do not compile. (There are a couple of definitions with additional dependencies, e.g. dagre and jquery, I have not added them so far, as I believe your publish process always adds those in as latest for missing explicit dependencies.)

The Travis tests also fail see here for the tests on my fork.

Given the tsconfig.json everything is resolved to ../ Any suggestions on how to proceed from here?

Note that I would definitely do an additional little chore commit to update the version numbers in comment headers of the affected definition files listed above, where they do not already have one. For alignment purposes, where possible.

@ghost
Copy link

ghost commented Oct 25, 2016

OK, turns out you do need to edit the tsconfig.

c3, d3-tip, d3.cloud.layout, nvd3, plottable

These packages only use d3, so just remove their dependency on the parent directory by removing baseUrl/types/typeRoots.

cal-heatmap, d3-kit, dagre-d3, dc

These depend on both node_modules and on some things from the parent directory. So we need to specify a hybrid typeRoots:

"typeRoots": [
    "node_modules/@types",
    "../"
],

@tomwanzek
Copy link
Contributor Author

Thx. I will touch up the tsconfig s today and see, if I can validate the versions for which the c3 etc. definitions are intended for to update their respective headers. We will need to do that, as some of them are in the process of being migrated from D3 v3 to D3 v4 as well. This way they should be able to be brought into line over time.

@ghost
Copy link

ghost commented Oct 25, 2016

Also, use caret ranges instead of ">=3 <4".

@tomwanzek
Copy link
Contributor Author

Can do, no problem.

@tomwanzek
Copy link
Contributor Author

Alright, I just pushed the resulting updates to my fork. Based on what I saw locally, as long as the travis tests run npm install in each affected folder, everything should work now.

I will check back and let you know, when Travis had some time to ponder the matter 😄

If there are no issues on my fork I will convert to a PR to DT/types-2.0.

@tomwanzek tomwanzek mentioned this issue Oct 25, 2016
9 tasks
@tomwanzek
Copy link
Contributor Author

@andy-ms As Travis was oh so happy on my fork, I have now formally submitted the PR #12256.

Once this one is through, we have arrived at the point where everything is simply about maintaining and improving the definitions in the ordinary course of business.

Thanks for all your hard work along the way, including @RyanCavanaugh @mhegazy .

@tomwanzek
Copy link
Contributor Author

So finally, with great pleasure I am closing this issue, as the D3 Standard Bundle now has a first version of the definitions published under version 4.2.37:

npm install @types/d3 --save now works officially.

There should be a follow-up to pin down the minor versions of the modules which it re-exports so that the standard bundle definition stays "roughly" aligned with the corresponding d3 bundle library. I.e. add a stub-package.json

This is all the more indicated, as 4.3.0 was just released, with some changes to d3-geo and d3-voronoi triggering the minor version bump. I have already opened separate issues to update the two module definitions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants