Replace D3 Legacy #12256

tomwanzek · 2016-10-25T21:56:29Z

This PR closes #11367. It is related to #9936. @andy-ms thanks for making this possible!!!

The d3 folder is updated to include a definition file for the Standard D3 version 4 bundle.
The definition file allows for using import-based scenarios and the use of a d3 global in vanilla scripts using the standard D3 version 4 bundle as a script. It simply re-exports the members of the constituent modules.

The legacy definitions have been renamed and left in the d3 folder or reference purposes.

In order to allow definitions of other libraries in DT depending on D3 version 3.5.x to still compile within DefinitelyTyped as well as to have the right dependencies when published to npm @types, the following changes where made:

add a package.json stub with the dependency on @types/d3: "^3.5.36" (This implies use of the latest published definition file for legacy D3). Dependencies on definitions for other libraries, e.g. jquery have so far not been added to the package.json. This means that, as before, the type-publisher process will add them in as latest.
the tsconfig.json files have been updated as necessary to ensure the definitions resolution respects the legacy definition version of D3 in the package.json
where missing and ascertainable, the version number of the affected library was added to the first line of the comments header as per convention for DT/types-2.0 to support version number parsing before publishing to npm @types
Each affected definition file has an added TODO comment advising of the need to check/update the D3 dependency, if the library gets updated to use D3 version 4.

No other changes were made to the affected library definitions.

The following definitions have been updated as per above:

EDIT: Corrected issue reference to 9936 in opening paragraph.

* Replaces the legacy D3 v3 definitions in `d3` with a definition file representing the D3 version 4 standard bundle * Add `package.json` file with legacy dependency to @types/d3 version >=3.5.36 <4.0.0

* Updated tsconfig.json to control typings resolution for D3 v3 * Updated package.json to use caret notation of D3 typings * Updated certain affected definitions header comments with version numbers, where version number was missing buit seemed reasonably ascertainable as latest. * Added comments to each affected definition file with TODO once upgrade to D3 v4 is considered

ghost · 2016-10-25T22:58:43Z

Don't see the relation to #9937, did you mean something else?

tomwanzek · 2016-10-25T23:02:48Z

@andy-ms Yes, thanks for catching my typo, I fixed it above.

tomwanzek · 2016-10-25T23:08:57Z

Travis seems to be a bit moody. Doesn't seems to want to start the job. Is there anything required from my side to trigger it again?

tomwanzek · 2016-10-26T01:12:15Z

🎉

tomwanzek · 2016-10-26T18:09:41Z

@andy-ms when you have a chance to merge this PR can you also kindly look at #12272 . It's a tiny fix PR for d3-format (it already passed Travis as well). Thanks!!! most appreciated.

ghost · 2016-10-26T18:11:37Z

c3/index.d.ts

@@ -1,10 +1,17 @@
-// Type definitions for C3js
+// Type definitions for C3js v0.4.11


No need for a patch version

I'll remove it. As an FYI, I tended to keep the patch version in for the d3-xxxxx definitions for ttwo reasons:
(1) It's easier to go back to the underlying library and check for changes in commits after the last validated patch w.r.t. definition accuracy, and
(2) which goes hand in hand with (1), while a patch release in JS may not break the JS API, there are times when it does break a strongly typed TS API definition.

ghost · 2016-10-26T18:17:41Z

cal-heatmap/tsconfig.json

Use the same indent it had before.

FYI. The .editorconfig I have in my fork specs out indent=2 for json files? So the changes were simply automatically applied.

ghost · 2016-10-26T18:18:48Z

d3-tip/package.json

@@ -0,0 +1,5 @@
+{
+  "dependencies": {


Prefer 4 space indents.

ghost · 2016-10-26T18:21:09Z

d3/legacy/plugins/package.json

@@ -0,0 +1,5 @@
+{


This file won't actually do anything right now -- legacy will be ignored.

That's ok as far as I can tell,it certainly does not break any dependencies in DT. If someone runs into an issue when newly acquiring the legacy definitions, we will hear about it and cross that bridge then...

ghost · 2016-10-26T18:22:27Z

nvd3/index.d.ts

@@ -1,9 +1,15 @@
-// Type definitions for nvd3 1.8.1
+// Type definitions for nvd3 v1.8.1


The v isn't needed.

Just a convention I used, no problem I can take it out.

tomwanzek · 2016-10-26T19:07:01Z

Pushed the updated changes as per review above. Travis is not starting...yet.

tomwanzek · 2016-10-27T15:42:07Z

@andy-ms Seems all in good working order now. Let me know, if you need anything else from my side. Cheers.

ghost · 2016-10-27T15:52:58Z

I think you should remove the TODO comments. I don't want people installing types and seeing a TODO comment at the top -- they'll assume it's a work in progress, when it's are actually fine as-is. For example, c3 currently uses d3 version 3.5, so the typings shouldn't be updated yet anyway.

The practice of updating the version number and updating dependencies should be common practices anyway, so we should catch it during code review.

tomwanzek · 2016-10-27T16:43:53Z

@andy-ms as per your request the comments have been removed. I am assuming that Travis will be finished in appr. 1.5 hrs.

So hopefully, within the next 2hrs we finally have a merge and consecutive publication to the npm @types organization.

The publication of the Standard Bundle definition for version 4 has been a long time coming... Thx in advance for proceeding with this!

ghost · 2016-10-27T17:23:11Z

One last thing -- could you eliminate any changes that only add some newlines to a file? I think those were previously the TODO comments.

tomwanzek · 2016-10-27T18:04:42Z

@andy-ms While I respect your vigilance, this particular PR process has become rather tedious and protracted for increasingly limited value.

We have worked for months to get these definitions published, with an extreme amount of patience for all the stumbling blocks and delays on the way. I understand the MSFT team, including yourself, are working very hard to provide great tooling. So are others who provide the content.

Tooling alone does not make for a smooth process.

Please confirm that there are no further changes required to get this PR published before I consider deleting added lines. Thank you.

cc @RyanCavanaugh

ghost · 2016-10-27T18:26:21Z

Yes, this is the last review from me (assuming there are no mistakes made in the next change).

tomwanzek · 2016-10-27T19:08:21Z

Final changes submitted.

tomwanzek · 2016-10-27T20:35:33Z

Final changes passed.

tomwanzek · 2016-10-27T20:37:17Z

Thx!

Ciwan1859 · 2016-10-27T22:53:15Z

@tomwanzek, I'm confused! When I go here, is that Type Definitions for D3 v4? or the older v3?

eamodio · 2016-10-27T23:03:01Z

@Ciwan1859 you have to look here https://github.com/DefinitelyTyped/DefinitelyTyped/tree/types-2.0/d3

Only the types-2.0 branch has been updated (as that is what gets pushed to npm)

tomwanzek · 2016-10-27T23:07:18Z

@Ciwan1859 in short as @eamodio just said. With a little background:

All definitions for D3 version 4 are contained in the DefinitelyTyped/types-2.0 branch as they are written for and with TypeScript 2. So they are contained here. These are the definitions you will acquire when you use TypeScript 2 and acquire definitions in the new 'normal' way by:

npm install @types/d3 --save

I.e. DefinitelyTyped/types-2.0 is currently the default repo which feeds the publication to npm @types.

The definitions in the old master branch are still the old (to be phased out) D3 v3.

You can look at #9936 for the history of these changes for D3 and the recently updated DefinitelyTyped Readme on the main page for the general background.

Ciwan1859 · 2016-10-28T07:29:26Z

Thanks guys, I'm still a n00b to Git and GitHub and only just noticed that Branch drop down in the top-left.

Up to this point, I've been manually adding the definitions files. I'll try your command and see if it works!

Thanks again.

Ciwan1859 · 2016-10-28T08:16:05Z

@tomwanzek does the --ambient flag work on the npm install @types/d3 --ambient?

tomwanzek · 2016-10-28T14:30:35Z

@Ciwan1859 the --ambient flag you are likely referring to is a legacy of using typings to install definitions from DefinitelyTyped (TypeScript 1.8.x).

If you are installing the d3 definitions with npm install @types/d3 --save alongside the actual D3 standard bundle npm install d3 --save, then you can import from the d3 module in all the usual TypeScript/ES6 ways: import * as d3 from 'd3';, import {select, Selection, arc} from 'd3'; etc.

If you want to use the standard bundle definitions from @types/d3 in a vanilla script (i.e. without a module loader), then you can access the d3 global by including:

/// <reference types='d3' />

d3.select('svg');  // Or whatever else your heart desires
...

In this latter case, you will have to include the d3 v4 bundle script into your HTML, rather than install/import it.

One note of caution, the vanilla script use is only supported out-of-the-box by the definitions for the standard bundle in @types/d3. The individual module definitions (i.e. d3-selection etc.) do not support it out-of-the-box.

Hope this helps.

tomwanzek added 2 commits October 24, 2016 12:09

Replace D3 v3 definition with D3 v4 (bundle)

cae6e5e

* Replaces the legacy D3 v3 definitions in `d3` with a definition file representing the D3 version 4 standard bundle * Add `package.json` file with legacy dependency to @types/d3 version >=3.5.36 <4.0.0

tomwanzek mentioned this pull request Oct 25, 2016

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

Closed

ghost reviewed Oct 26, 2016

View reviewed changes

Chore changes as per review.

58c535d

Removed TODO "upgrade-to-v4" comments

32c2416

Removed added empty lines.

7a92849

ghost merged commit 0500bd3 into DefinitelyTyped:types-2.0 Oct 27, 2016

tomwanzek deleted the replace-d3-legacy branch October 27, 2016 20:53

tomwanzek mentioned this pull request Oct 27, 2016

Request for d3.js API update from v3 to v4.1.0 #9936

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace D3 Legacy #12256

Replace D3 Legacy #12256

tomwanzek commented Oct 25, 2016 •

edited

Loading

ghost commented Oct 25, 2016

tomwanzek commented Oct 25, 2016

tomwanzek commented Oct 25, 2016

tomwanzek commented Oct 26, 2016

tomwanzek commented Oct 26, 2016

ghost Oct 26, 2016

tomwanzek Oct 26, 2016

ghost Oct 26, 2016

tomwanzek Oct 26, 2016

tomwanzek Oct 26, 2016

ghost Oct 26, 2016

ghost Oct 26, 2016

tomwanzek Oct 26, 2016

ghost Oct 26, 2016

tomwanzek Oct 26, 2016

ghost Oct 26, 2016

tomwanzek Oct 26, 2016

tomwanzek commented Oct 26, 2016

tomwanzek commented Oct 27, 2016

ghost commented Oct 27, 2016

tomwanzek commented Oct 27, 2016

ghost commented Oct 27, 2016 •

edited by ghost

Loading

tomwanzek commented Oct 27, 2016

ghost commented Oct 27, 2016

tomwanzek commented Oct 27, 2016

tomwanzek commented Oct 27, 2016

tomwanzek commented Oct 27, 2016

Ciwan1859 commented Oct 27, 2016

eamodio commented Oct 27, 2016

tomwanzek commented Oct 27, 2016

Ciwan1859 commented Oct 28, 2016

Ciwan1859 commented Oct 28, 2016

tomwanzek commented Oct 28, 2016

		@@ -1,10 +1,17 @@
		// Type definitions for C3js
		// Type definitions for C3js v0.4.11

		@@ -1,9 +1,15 @@
		// Type definitions for nvd3 1.8.1
		// Type definitions for nvd3 v1.8.1

Replace D3 Legacy #12256

Replace D3 Legacy #12256

Conversation

tomwanzek commented Oct 25, 2016 • edited Loading

ghost commented Oct 25, 2016

tomwanzek commented Oct 25, 2016

tomwanzek commented Oct 25, 2016

tomwanzek commented Oct 26, 2016

tomwanzek commented Oct 26, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomwanzek commented Oct 26, 2016

tomwanzek commented Oct 27, 2016

ghost commented Oct 27, 2016

tomwanzek commented Oct 27, 2016

ghost commented Oct 27, 2016 • edited by ghost Loading

tomwanzek commented Oct 27, 2016

ghost commented Oct 27, 2016

tomwanzek commented Oct 27, 2016

tomwanzek commented Oct 27, 2016

tomwanzek commented Oct 27, 2016

Ciwan1859 commented Oct 27, 2016

eamodio commented Oct 27, 2016

tomwanzek commented Oct 27, 2016

Ciwan1859 commented Oct 28, 2016

Ciwan1859 commented Oct 28, 2016

tomwanzek commented Oct 28, 2016

tomwanzek commented Oct 25, 2016 •

edited

Loading

ghost commented Oct 27, 2016 •

edited by ghost

Loading