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
update to d3@7.4 #60427
update to d3@7.4 #60427
Conversation
https://github.com/d3/d3-shape/releases/tag/v3.1.0 Add d3.symbolsStroke. Add d3.symbolsFill, replacing (deprecating) d3.symbols. Add d3.symbolAsterisk. Add d3.symbolDiamond2. Add d3.symbolPlus. Add d3.symbolSquare2. Add d3.symbolTriangle2. Add d3.symbolX. Add d3.link.
@Fil Thank you for submitting this PR! I see this is your first time submitting to DefinitelyTyped 👋 — I'm the local bot who will help you through the process of getting things through. This is a live comment which I will keep updated. 5 packages in this PR
@Fil: I see that you have added yourself as an owner to several packages, are you sure you want to become an owner? Code ReviewsBecause this is a widely-used package, a DT maintainer will need to review it before it can be merged. You can test the changes of this PR in the Playground. Status
All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes. Diagnostic Information: What the bot saw about this PR{
"type": "info",
"now": "-",
"pr_number": 60427,
"author": "Fil",
"headCommitOid": "35db98e4645375652dffaa4d04d2b4a3799dd3d7",
"mergeBaseOid": "8ddbf19efe34c53726e06fac519da853b8360af8",
"lastPushDate": "2022-05-20T14:12:21.000Z",
"lastActivityDate": "2022-05-22T15:19:10.000Z",
"mergeOfferDate": "2022-05-20T19:46:09.000Z",
"mergeRequestDate": "2022-05-22T15:19:10.000Z",
"mergeRequestUser": "Fil",
"hasMergeConflict": false,
"isFirstContribution": true,
"tooManyFiles": false,
"hugeChange": false,
"popularityLevel": "Critical",
"pkgInfo": [
{
"name": "d3-array",
"kind": "edit",
"files": [
{
"path": "types/d3-array/d3-array-tests.ts",
"kind": "test"
},
{
"path": "types/d3-array/index.d.ts",
"kind": "definition"
}
],
"owners": [
"gustavderdrache",
"borisyankov",
"tomwanzek",
"denisname",
"ledragon",
"Methuselah96"
],
"addedOwners": [
"Fil"
],
"deletedOwners": [],
"popularityLevel": "Popular"
},
{
"name": "d3-color",
"kind": "edit",
"files": [
{
"path": "types/d3-color/d3-color-tests.ts",
"kind": "test"
},
{
"path": "types/d3-color/index.d.ts",
"kind": "definition"
}
],
"owners": [
"tomwanzek",
"gustavderdrache",
"borisyankov",
"denisname",
"ledragon",
"Methuselah96"
],
"addedOwners": [
"Fil"
],
"deletedOwners": [],
"popularityLevel": "Critical"
},
{
"name": "d3-hierarchy",
"kind": "edit",
"files": [
{
"path": "types/d3-hierarchy/d3-hierarchy-tests.ts",
"kind": "test"
},
{
"path": "types/d3-hierarchy/index.d.ts",
"kind": "definition"
}
],
"owners": [
"tomwanzek",
"gustavderdrache",
"borisyankov",
"denisname",
"Methuselah96"
],
"addedOwners": [
"Fil"
],
"deletedOwners": [],
"popularityLevel": "Popular"
},
{
"name": "d3-shape",
"kind": "edit",
"files": [
{
"path": "types/d3-shape/d3-shape-tests.ts",
"kind": "test"
},
{
"path": "types/d3-shape/index.d.ts",
"kind": "definition"
}
],
"owners": [
"tomwanzek",
"gustavderdrache",
"borisyankov",
"denisname",
"Methuselah96"
],
"addedOwners": [
"Fil"
],
"deletedOwners": [],
"popularityLevel": "Critical"
},
{
"name": "d3",
"kind": "edit",
"files": [
{
"path": "types/d3/index.d.ts",
"kind": "definition"
}
],
"owners": [
"tomwanzek",
"gustavderdrache",
"borisyankov",
"denisname",
"Methuselah96"
],
"addedOwners": [
"Fil"
],
"deletedOwners": [],
"popularityLevel": "Popular"
}
],
"reviews": [
{
"type": "approved",
"reviewer": "peterblazejewicz",
"date": "2022-05-20T19:45:26.000Z",
"isMaintainer": true
},
{
"type": "approved",
"reviewer": "Methuselah96",
"date": "2022-05-20T14:23:23.000Z",
"isMaintainer": false
}
],
"mainBotCommentID": 1131480302,
"ciResult": "pass"
} |
🔔 @gustavderdrache @borisyankov @tomwanzek @denisname @Ledragon @Methuselah96 — please review this PR in the next few days. Be sure to explicitly select |
Hey @Fil, glad to see you over here. The pattern I've tried to follow in the past is to one minor version of d3 per PR. That way we don't miss adding anything from previous versions and the d3 version number is as close to accurate as possible. For example, this PR adds stuff from d3 7.3 even though the published version of d3 will still only be 7.1 if this is merged. As an example of how I've done it in the past here is: 6.4, 6.5, 6.6, 6.7, and 7.1. If you're interested in maintaining the types consistently like that, then that would be awesome. Otherwise, I can try to pick back up the work and get the latest few versions done. |
Meaning, if I want to update d3-shape to @3.1 (as I did here), I also need to upgrade the types for all the other modules that have had changes between d3@7.1 (last release that was "typed") and d3@7.4 (current version of D3)? If this is correct, and double-checking https://github.com/d3/d3/releases, this means I need to update:
right? |
Right, although you don't need to go to 7.4, since these d3-shape changes were made in 7.3. So the PRs I would imagine would be in this order:
I'm totally willing to do the other PRs myself, this is just the easiest way for us to make sure we aren't missing any type changes. Let me know if you're up for doing the earlier versions or if you would like me to. |
This comment was marked as outdated.
This comment was marked as outdated.
d3.rank can now take a comparator in addition to an accessor We don't need to change anything for "d3.rank, d3.sort, d3.bisector, and d3.groupSort now require comparators to have exactly two arguments", since that's how it was already typed (even though the code was actually more lax).
d3-array done! I'm stopping for now |
Inspecting the JavaScript source for this package found some properties that are not in the .d.ts files. d3-array (unpkg)was missing the following properties:
d3-hierarchy (unpkg)was missing the following properties:
|
what is this missing "scan" property? 👀 |
note: I'm not sure that the path function must have 3 arguments; most of the time people will use only 1 argument
- add rgb.clamp and hsl.clamp - add color.formatHex8
@Methuselah96 : I've finally done the four modules.
|
It analyzes content of the module, it can show false positives at some scanerios. In short, it detects exported symbols in a module and compares with exported one from types. You can skip this one as a niptick, thx! |
The parameters for callback functions that a user passes into a method are always optional and don't need to be marked as such. The user can just pass in a function that has no parameters and TypeScript considers that to match a function signature with parameters.
No, this is fine, thanks for putting in the work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also bump the version numbers in d3/index.d.ts
?
@Fil One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits. Thank you! |
@Methuselah96 Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks for your work on this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Fil thanks!
Ready to merge |
Updates:
d3-array
https://github.com/d3/d3-array/releases/tag/v3.1.6
d3.rank can now take a comparator in addition to an accessor
d3.rank, d3.sort, d3.bisector, and d3.groupSort now require comparators to have exactly two arguments.
d3-shape
https://github.com/d3/d3-shape/releases/tag/v3.1.0
Add d3.symbolsStroke.
Add d3.symbolsFill, replacing (deprecating) d3.symbols.
Add d3.symbolAsterisk.
Add d3.symbolDiamond2.
Add d3.symbolPlus.
Add d3.symbolSquare2.
Add d3.symbolTriangle2.
Add d3.symbolX.
Add d3.link.
d3-hierarchy
https://github.com/d3/d3-hierarchy/releases/tag/v3.1.2
Add stratify.path.
d3-color
https://github.com/d3/d3-color/releases/tag/v3.1.0
Add rgb.clamp and hsl.clamp. d3/d3-color#102
Add color.formatHex8. d3/d3-color#103
Please fill in this template.
npm test <package to test>
.Select one of these and delete the others:
If adding a new definition:
.d.ts
files generated via--declaration
If changing an existing definition: