Closed jiemakel closed 7 years ago
@jiemakel Thanks for the comment and proposed PR. You are correct, that the D3 API docs are explicit in some cases about the return type for a well-defined 'failure mode', e.g. null
or undefined
. Unfortunately, I did not yet have the time to do a complete sweep through the source code to confirm comprehensively where methods return with | null
or | undefined
or even | null | undefined
. I.e. for those where the API doc is not explicit.
As for the compiler option, the above is the reason, why I have so far left it as false
. I did not want to give the impression, that the definitions are strictly speaking strict yet.
Before I consider merging, let me consider the broader implication of completing the strictness verification across all modules consistently.
As opposed to DefinitelyTyped, this repo uses a single compilation context, so flicking the switch would affect all modules equally. This is a bit of a historic artifact, of how this repo evolved.
I'll respond more completely shortly. Cheers.
@jiemakel my apologies for the delay in getting back to this. With respect to this repo, I prioritized the migration to DT/types-2.0 and npm @types by implication. There were a couple of hurdles to take, so that (a) the production version of the definitions is accessible to the masses (with correct version numbers on npm), and (b) there is less effort in synchronizing the changes while the migration is ongoing.
That being said, for all intents and purposes the migration is complete for all the standard bundle D3 modules. So, my preference would be to move this open item over to DefinitelyTyped. The current main tracking issue on DT is 9936 for your reference.
So as part of the migration, I will move this issue over with the considered changes. The benefit is also, that we could work through it on a module-by-module basis, validate the API w.r.t to strictNullChecking, enhance it, update tests and JSDoc comments.
The latter is another one of these to-dos, best handled in DT now.
Hope this is agreeable. I will copy you when I move this item over to DT. In the meantime, I will leave this open for reference. Thx.
As discussed, the issue has been migrated to DefinitelyTyped as a work item under issue 11365
@jiemakel I will leave this PR open, until your specific proposed changes could be migrated into PRs on DefinitelyTyped against the types-2,0
branch. Those should be the source of truth from now on.
See DT PR 12858 for submission of d3-selection, d3-transition, d3-selection-multi and d3-force as first batch. At long last... :smile:
The general tracking issue remains: DT Issue 11365
I will close this PR now, given the actioned follow-up after catering to some other priorities in between. Cheers, T
I want to use the d3 v4 definitions (thank you very much for doing them BTW) in a project that has
--strictNullChecks
enabled. At certain points, the d3 definitions themselves made references tonull
s andundefined
s that caused errors in this mode. This PR adds the requirednull
/undefined
union types to the d3 definitions that make these errors go away (but don't affect usage in any way in non strict null checking mode).What is not in this PR are any tests to make sure that compilation under
--strictNullChecks
succeeds. I tried to fiddle around with the Gruntfile a bit to create such, but for some reason changing thestrictNullChecks
option there totrue
there did nothing, so in the end I just abandoned the effort altogether.