@denisname Thank you for submitting this PR!

🔔 @tomwanzek @gustavderdrache @borisyankov - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead.

@denisname The Travis CI build failed! Please review the logs for more information. Once you've pushed the fixes, the build will automatically re-run. Thanks!

@denisname Thank you for submitting this PR. Given the extent of the contemplated changes, I will take some time to review them. In particular, since at first glance, you are proposing diverting from some of the conventions we have used in writing previous definitions. I will explain in my comments.

I will take the time to digest and respond with motivations for some of the conventions we followed in the past. Should we reconsider some of them, I would prefer to have full alignment first and then update other definitions as appropriate for consistency.

So that being said, it might be worthwhile to hold off on your proposed efforts with d3-array, until we have such alignment. This will save cycles for you and reviewers, should there be items where we prefer to hold on to current conventions. With 30+ definitions in the core D3 universe, conventions matter for a consistent user experience.

Thanks in advance for your understanding.

Ok. Note that I cut my changes into several small commits to make revision simpler.

As for a minimal constraint on the generic Domain, we could define:

export type AxisDomain = number | string | Date | { valueOf(): number};

and then impose:

export interface AxisScale<Domain extends AxisDomain> {...}
...
export interface Axis<Domain extends AxisDomain> { ... }
...
export function axisTop<Domain extends AxisDomain>(scale: AxisScale<Domain>): Axis<Domain>;
export function axisRight<Domain extends AxisDomain>(scale: AxisScale<Domain>): Axis<Domain>;
export function axisBottom<Domain extends AxisDomain>(scale: AxisScale<Domain>): Axis<Domain>;
export function axisLeft<Domain extends AxisDomain>(scale: AxisScale<Domain>): Axis<Domain>;

Defaulting the generic for Axis and AxisScale to AxisDomain turns out to be impractical due to the impact of strictFunctionChecks.

@denisname Penny for your thoughts...:smile:

• numerals?: [string, string, ..., string]
  Fair point about assigning a string[]-inferred-type.

• format[Prefix]
  Let's go for (n: number | { valueOf(): number }) => string.

• strict function types test
  Removed!

• scale generic:
  While making the changes, I realize that the tests line 80 and 87 are no failing as expected. The errors are caused by reassigning a const and not because there is a generic missing. The expected error stop happening with TS 2.4 which "improved inference for generics". I'm not sure what to do with the two tests. Best is probably to remove them!?

  I take this opportunity to say also that I disagree about scale being generic. See getMeAT for reasons. Also, if you don't like as keyword, <ScaleOrdinal<string, number>>bottomAxis.scale() is same number of characters than bottomAxis.scale<ScaleOrdinal<string, number>>(). One more with a space. But, I recognise the importance of keeping the different d3 definitions consistent and to avoid breaking backward compatibility. And as TS 2.4 makes the generic "optional", I'm going to set the generic of scale back… 😊

Edit: CI build failed as expected, see "scale generic".

The AxisDomain constraint seems good. 👍

I also tried to solve the problem of ticks et ticksFormat typing and to remove: tickArguments(args: any[]): this;. Because of that overloading, any array is accepted by the compiler for tickArguments...

For this, I separated AxisScale in two (see full code)

export interface AxisScale<Domain> {
    (x: Domain): number | undefined;
    domain(): Domain[];
    range(): number[];
    copy(): this;
    bandwidth?(): number;
}

export interface TickedAxisScale<Domain, Tick> extends AxisScale<Domain> {
    ticks(count?: Tick): Domain[];
    tickFormat(count?: Tick, specifier?: string): ((d: Domain) => string);
}

I then removed ticks(.) and ticksFormat(.) from Axis<Domain>, and added:

export interface TickedAxis<Domain, Tick> extends Axis<Domain> {
    // ...
    ticks(count: Tick, specifier?: string): this;
    tickArguments(): [Tick] | [Tick, string];
    tickArguments(args: [Tick] | [Tick, string]): this;
}

export function axisTop<Domain, Tick>(scale: TickedAxisScale<Domain, Tick>): TickedAxis<Domain, Tick>;
export function axisRight<Domain, Tick>(scale: TickedAxisScale<Domain, Tick>): TickedAxis<Domain, Tick>;
export function axisBottom<Domain, Tick>(scale: TickedAxisScale<Domain, Tick>): TickedAxis<Domain, Tick>;
export function axisLeft<Domain, Tick>(scale: TickedAxisScale<Domain, Tick>): TickedAxis<Domain, Tick>;

Now type inference detect which types are allowed for ticks(.) and ticksFormat(.), see gist for all tests. For example:

d3Axis.axisTop(scaleLinear()).ticks(1, ',f'); // Ok
d3Axis.axisTop(scaleLinear()).ticks(timeMinute.every(5)!, ',f'); // fails

Provided that, in d3-scale, the two ticks(.) are merged: ticks(count: number | TimeInterval): Date[];.

In the gist I also added TickedCustomAxisScale. But, are there really any people who create scales that have a singnature for ticks and ticksFormat other than number or TimeInterval!? And if that's the case, is it not better for them to define their own adapted interface, rather than using TickedCustomAxisScale?

Hope my explanations are clear. Waiting for your comments. :)

@denisname Thanks, I will respond at my earliest. Unfortunately, I am little otherwise-committed right now 😄

Thanks for your contribution. This PR has failing CI tests and can not be merged in at the time being. For housekeeping purposes we are closing stale PRs. If you'd still like to merge this code in, please open a new PR that has been merged and rebased with the master branch.

@denisname My apologies for not having been faster than, it's been a little hectic. I would love to resurrect your work and get it going.

There are a few options (bar reopening this PR and re-basing it):
(1) You...
(2) I...
create a new PR with the latest state of this one and cross-reference.
The first option would clearly preserve your line contributions. Also, I would suggest you add yourself
to the maintainer listing in the header, as you are clearly committed to moving this and other D3 definitions forward and make quality contributions.

Then we take it from there...

As for the the specific comment on scale<...>() vs scale() as ... I take your point that it is recommended practice. Given implications for backwards-compatibility, I would suggest we go with:

scale<A extends AxisScale<Domain> = AxisScale<Domain>>(): A;

for now. It will allow scale() to work in conjunction with as without the need for casting the method.
Gentle off-ramp for a future change to using your original proposal.

These kind of changes have been a bit of a thorn in our side, because we cannot semantically version the definitions in their own right. I will create an RFC item to discuss something that has been on my mind for a while now. I will @-mention you alongside @gustavderdrache and @Ledragon as key stakeholders.

PS: Ignore the comment suggesting to add the default to the generic in scale<...>(). Of course, that is not needed, as the constraint will form the default in this case.

As for the lines that create problems for // $ExpectError, for one, if a const should be a let, let's fix that. That was the issue with let creating a linter error, if the re-assignment line is commented out.

If we are still running into issues where the error assertion does not work in some TS version, but works in others, maybe we should just remove that particular definitions test.