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

Possible limitations of *.d.ts files #18791

Closed
johnnyreilly opened this issue Sep 27, 2017 · 5 comments
Closed

Possible limitations of *.d.ts files #18791

johnnyreilly opened this issue Sep 27, 2017 · 5 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@johnnyreilly
Copy link

johnnyreilly commented Sep 27, 2017

Hello!

I'm a member of the Definitely Typed team and I'm confused. I've spent a number of hours trying out various scenarios / solutions. All have died like tears in rain. So I turn to you good folks of TypeScript; set me straight! I'm implore you; right my addled brain. Or tell me that I'm right; there's limitations when it comes to defining definition files.

The story begins here:

Once upon a time (yesterday) we decided to use big.js in our project. It's popular and my old friend @nycdotnet apparently originally wrote the type definitions which can be found here. They were updated by @googol a couple of days ago.

Our project uses webpack to bundle and I thought we'd be off to the races there and then. We weren't.
There's a couple of problems that I encountered. Late me take them in turn.

UMD / CommonJS and Global exports oh my!

My usage code was as simple as this:

import * as BigJs from 'big.js';
const lookABigJs = new BigJs(1);

If you execute it in a browser it works. It makes me a Big. However the TypeScript compiler is not happy. No siree. Nope. It's bellowing at me:

[ts] Cannot use 'new' with an expression whose type lacks a call or construct signature.

So I think; huh! I guess @googol just missed something off when he updated the definition files. No bother. I'll fix it. I take a look at how big.js exposes itself to the outside world. Thusly:

    //AMD.
    if (typeof define === 'function' && define.amd) {
        define(function () {
            return Big;
        });
        
    // Node and other CommonJS-like environments that support module.exports.
    } else if (typeof module !== 'undefined' && module.exports) {
        module.exports = Big;
        module.exports.Big = Big;
    //Browser.
    } else {
        global.Big = Big;
    }

Now, webpack is supersmart; it can take all kinds of module formats. That includes AMD / UMD. So when webpack encounters the above code it rolls with the AMD branch and the import * as BigJs from 'big.js'; lands up resolving to the return Big; above. Big is essentially the constructor function of big.js. I took a look at the relevant portion of the definition file I find this:

export const Big: BigConstructor;

Which tells me that Big is being exported as a subproperty of the definition. I do a little dance with options that might occur to me; adding a export = Big seems like a good option. Hey! It works in my local project; success! So I fork DT and add my amend to the definition file only to be confronted by this:

[ts] An export assignment cannot be used in a module with other exported elements.

Huh. Now I'm stuck. It occurs to try something different.

Static .... constructors?

I dig my way through the various examples and look for the example closest to what big.js actually is:

https://www.typescriptlang.org/docs/handbook/declaration-files/templates.html

The most obvious candidate is the module-class.d.ts template. big.js is essentially a class so I should rewrite the definition file accordingly. I do. Look at the first lines below and you will see a problem:

export as namespace Big;
export = Big;

// CANNOT MODEL THESE.....

/**
 * Returns a new instance of a Big number object
 *
 * String values may be in exponential, as well as normal (non-exponential) notation.
 * There is no limit to the number of digits of a string value (other than that of Javascript's maximum array size), but the largest recommended exponent magnitude is 1e+6.
 * Infinity, NaN and hexadecimal literal strings, e.g. '0xff', are not valid.
 * String values in octal literal form will be interpreted as decimals, e.g. '011' is 11, not 9.
 *
 * @throws `NaN` on an invalid value.
 */
// (value: BigSource): Big;

/**
 * Create an additional Big number constructor
 *
 * Values created with the returned constructor will have a separate set of configuration values.
 * This can be used to create Big objects with different DP and RM values.
 * Big numbers created by different constructors can be used together in operations, and it is the DP and RM setting of the Big number that an operation is called upon that will apply.
 * In the interest of memory efficiency, all Big number constructors share the same prototype object,
 * so while the DP and RM (and any other own properties) of a constructor are isolated and untouchable by another, its prototype methods are not.
 */
// (): BigConstructor;


declare class Big {
    /**
     * Returns a new instance of a Big number object
     *
     * String values may be in exponential, as well as normal (non-exponential) notation.
     * There is no limit to the number of digits of a string value (other than that of Javascript's maximum array size), but the largest recommended exponent magnitude is 1e+6.
     * Infinity, NaN and hexadecimal literal strings, e.g. '0xff', are not valid.
     * String values in octal literal form will be interpreted as decimals, e.g. '011' is 11, not 9.
     *
     * @throws `NaN` on an invalid value.
     */
    constructor(value: Big.BigSource);

    /**
     * The maximum number of decimal places of the results of operations involving division.
     * It is relevant only to the div and sqrt methods, and the pow method when the exponent is negative.
     *
     * 0 to 1e+6 inclusive
     * Default value: 20
     */
    static DP: number;
    /**
     * The rounding mode used in the above operations and by round, toExponential, toFixed and toPrecision.
     * Default value: 1
     */
    static RM: number;
    /**
     * The negative exponent value at and below which toString returns exponential notation.
     *
     * -1e+6 to 0 inclusive
     * Default value: -7
     */
    static E_NEG: number;
    /**
     * The positive exponent value at and above which toString returns exponential notation.
     *
     * 0 to 1e+6 inclusive
     * Default value: 21
     */
    static E_POS: number;

    /** Returns a Big number whose value is the absolute value, i.e. the magnitude, of this Big number. */
    abs(): Big;
    /**
     * Compare the values.
     *
     * @throws `NaN` if n is invalid.
     */
    cmp(n: Big.BigSource): Big.Comparison;
    /**
     * Returns a Big number whose value is the value of this Big number divided by n.
     *
     * If the result has more fraction digits than is specified by Big.DP, it will be rounded to Big.DP decimal places using rounding mode Big.RM.
     *
     * @throws `NaN` if n is invalid.
     * @throws `±Infinity` on division by zero.
     * @throws `NaN` on division of zero by zero.
     */
    div(n: Big.BigSource): Big;
    /**
     * Returns true if the value of this Big equals the value of n, otherwise returns false.
     *
     * @throws `NaN` if n is invalid.
     */
    eq(n: Big.BigSource): boolean;
    /**
     * Returns true if the value of this Big is greater than the value of n, otherwise returns false.
     *
     * @throws `NaN` if n is invalid.
     */
    gt(n: Big.BigSource): boolean;
    /**
     * Returns true if the value of this Big is greater than or equal to the value of n, otherwise returns false.
     *
     * @throws `NaN` if n is invalid.
     */
    gte(n: Big.BigSource): boolean;
    /**
     * Returns true if the value of this Big is less than the value of n, otherwise returns false.
     *
     * @throws `NaN` if n is invalid.
     */
    lt(n: Big.BigSource): boolean;
    /**
     * Returns true if the value of this Big is less than or equal to the value of n, otherwise returns false.
     *
     * @throws `NaN` if n is invalid.
     */
    lte(n: Big.BigSource): boolean;
    /**
     * Returns a Big number whose value is the value of this Big number minus n.
     *
     * @throws `NaN` if n is invalid.
     */
    minus(n: Big.BigSource): Big;
    /**
     * Returns a Big number whose value is the value of this Big number modulo n, i.e. the integer remainder of dividing this Big number by n.
     *
     * The result will have the same sign as this Big number, and it will match that of Javascript's % operator (within the limits of its precision) and BigDecimal's remainder method.
     *
     * @throws `NaN` if n is negative or otherwise invalid.
     */
    mod(n: Big.BigSource): Big;
    /**
     * Returns a Big number whose value is the value of this Big number plus n.
     *
     * @throws `NaN` if n is invalid.
     */
    plus(n: Big.BigSource): Big;
    /**
     * Returns a Big number whose value is the value of this Big number raised to the power exp.
     *
     * If exp is negative and the result has more fraction digits than is specified by Big.DP, it will be rounded to Big.DP decimal places using rounding mode Big.RM.
     *
     * @param exp The power to raise the number to, -1e+6 to 1e+6 inclusive
     * @throws `!pow!` if exp is invalid.
     *
     * Note: High value exponents may cause this method to be slow to return.
     */
    pow(exp: number): Big;
    /**
     * Returns a Big number whose value is the value of this Big number rounded using rounding mode rm to a maximum of dp decimal places.
     *
     * @param dp Decimal places, 0 to 1e+6 inclusive
     * @param rm The rounding mode, one of the RoundingMode enumeration values
     * @throws `!round!` if dp is invalid.
     * @throws `!Big.RM!` if rm is invalid.
     */
    round(dp?: number, rm?: Big.RoundingMode): Big;
    /**
     * Returns a Big number whose value is the square root of this Big number.
     *
     * If the result has more fraction digits than is specified by Big.DP, it will be rounded to Big.DP decimal places using rounding mode Big.RM.
     *
     * @throws `NaN` if this Big number is negative.
     */
    sqrt(): Big;
    /**
     * Returns a Big number whose value is the value of this Big number times n.
     *
     * @throws `NaN` if n is invalid.
     */
    times(n: Big.BigSource): Big;
    /**
     * Returns a string representing the value of this Big number in exponential notation to a fixed number of decimal places dp.
     *
     * If the value of this Big number in exponential notation has more digits to the right of the decimal point than is specified by dp,
     * the return value will be rounded to dp decimal places using rounding mode Big.RM.
     *
     * If the value of this Big number in exponential notation has fewer digits to the right of the decimal point than is specified by dp, the return value will be appended with zeros accordingly.
     *
     * If dp is omitted, or is null or undefined, the number of digits after the decimal point defaults to the minimum number of digits necessary to represent the value exactly.
     *
     * @param dp Decimal places, 0 to 1e+6 inclusive
     * @throws `!toFix!` if dp is invalid.
     */
    toExponential(dp?: number): string;
    /**
     * Returns a string representing the value of this Big number in normal notation to a fixed number of decimal places dp.
     *
     * If the value of this Big number in normal notation has more digits to the right of the decimal point than is specified by dp,
     * the return value will be rounded to dp decimal places using rounding mode Big.RM.
     *
     * If the value of this Big number in normal notation has fewer fraction digits then is specified by dp, the return value will be appended with zeros accordingly.
     *
     * Unlike Number.prototype.toFixed, which returns exponential notation if a number is greater or equal to 1021, this method will always return normal notation.
     *
     * If dp is omitted, or is null or undefined, then the return value is simply the value in normal notation.
     * This is also unlike Number.prototype.toFixed, which returns the value to zero decimal places.
     *
     * @param dp Decimal places, 0 to 1e+6 inclusive
     * @throws `!toFix!` if dp is invalid.
     */
    toFixed(dp?: number): string;
    /**
     * Returns a string representing the value of this Big number to the specified number of significant digits sd.
     *
     * If the value of this Big number has more digits than is specified by sd, the return value will be rounded to sd significant digits using rounding mode Big.RM.
     *
     * If the value of this Big number has fewer digits than is specified by sd, the return value will be appended with zeros accordingly.
     *
     * If sd is less than the number of digits necessary to represent the integer part of the value in normal notation, then exponential notation is used.
     *
     * If sd is omitted, or is null or undefined, then the return value is the same as .toString().
     *
     * @param sd Significant digits, 1 to 1e+6 inclusive
     * @throws `!toPre!` if sd is invalid.
     */
    toPrecision(sd?: number): string;
    /**
     * Returns a string representing the value of this Big number.
     *
     * If this Big number has a positive exponent that is equal to or greater than 21, or a negative exponent equal to or less than -7, then exponential notation is returned.
     *
     * The point at which toString returns exponential rather than normal notation can be adjusted by changing
     * the value of Big.E_POS and Big.E_NEG. By default, Big numbers correspond to Javascript's number type in this regard.
     */
    toString(): string;
    /**
     * Returns a string representing the value of this Big number.
     *
     * If this Big number has a positive exponent that is equal to or greater than 21, or a negative exponent equal to or less than -7, then exponential notation is returned.
     *
     * The point at which toString returns exponential rather than normal notation can be adjusted by changing
     * the value of Big.E_POS and Big.E_NEG. By default, Big numbers correspond to Javascript's number type in this regard.
     */
    valueOf(): string;
    /**
     * Returns a string representing the value of this Big number.
     *
     * If this Big number has a positive exponent that is equal to or greater than 21, or a negative exponent equal to or less than -7, then exponential notation is returned.
     *
     * The point at which toString returns exponential rather than normal notation can be adjusted by changing
     * the value of Big.E_POS and Big.E_NEG. By default, Big numbers correspond to Javascript's number type in this regard.
     */
    toJSON(): string;
}


declare namespace Big {
    export type BigSource = number | string | Big;

    export const enum Comparison {
        GT = 1,
        EQ = 0,
        LT = -1,
    }

    export const enum RoundingMode {
        /**
         * Rounds towards zero.
         * I.e. truncate, no rounding.
         */
        RoundDown = 0,
        /**
         * Rounds towards nearest neighbour.
         * If equidistant, rounds away from zero.
         */
        RoundHalfUp = 1,
        /**
         * Rounds towards nearest neighbour.
         * If equidistant, rounds towards even neighbour.
         */
        RoundHalfEven = 2,
        /**
         * Rounds away from zero.
         */
        RoundUp = 3,
    }
}

Yup the constructor function exposed by big.js is more than just a constructor function. Alas.

https://github.com/MikeMcl/big.js#use

As you can see, the constructor function can be used to directly instantiate a Big (so Big(1) is legitimate usage). What's more there's this valid use case:

How can I simultaneously use different decimal places and/or rounding mode settings for different Big numbers?
From v3.0.0, it is possible to have multiple Big number constructors each with their own particular DP and RM settings which apply to all Big numbers created from it.

/*
Create an additional Big number constructor by calling the original Big
number constructor without using new and without any argument.
*/
Big10 = Big();

// Set the decimal places of division operations for each constructor.
Big.DP = 3;
Big10.DP = 10;

x = Big(5);
y = Big10(5);

x.div(3)     // 1.667
y.div(3)     // 1.6666666667

What's more, using the approach laid out in the template I can't expose Big as both the child export (for commonjs) as well as the default export for AMD and commonjs. So again I'm toast.

Help me, I'm dying

I'm not; I know. I'm sure this is possible and I'm just missing how.

But it definitely (ha!) seems that there's some holes in the definition file templates that are in need of filling. Could you answer this question and hopefully we can improve the templates off the back of it. I'm happy to help with that if I can.

There's a related comment here which may prove relevant: DefinitelyTyped/DefinitelyTyped#3548 (comment) However; it's kind of old and so I'm not sure if it still applies.

As mentioned; I'm probably the idiot. But if I'm confused I guarantee others are. Please help me.

To Summarise

  1. In a definition file how can we expose something as AMD / UMD, CommonJs (both nested and default) and a global exports?
  2. How can we model classes that are more than they seem? Are we limited to decomposed classes (i.e. as the existing definitions use)? If so, how can we expose decomposed classes as a single class as described in 1?
@kitsonk
Copy link
Contributor

kitsonk commented Sep 27, 2017

In the case above, there is a different shape depending on which loader is being used and I am not sure that makes a huge amount of sense. The AMD shape is different than the CommonJS shape. While that is technically possible, that feels like that is an issue.

Under AMD, the shape would be just Big while under CommonJS the shape of the module would have a reference to itself under the property Big.Big. While they have likely done that for some sort of compatibility, it means that the types change depending on loader use. Also, falling back to modifying the global also has some issues, in that global is yet to be approved as a spec and would only work in certain run-time environments.

Instead of re-writing as classes (when they aren't) you could do something like this, which is what we do for Dojo 1, which is AMD. If there are weird shapes, we deal with it in the global namespace. We haven't had the time/desire to move to UMD type definitions:

declare module 'dijit/form/FilteringSelect' {
	type FilteringSelect<C extends dijit.form.Constraints, T extends Object, Q extends string | Object | Function, O extends dojo.store.api.QueryOptions> = dijit.form.FilteringSelect<C, T, Q, O>;
	const FilteringSelect: dijit.form.FilteringSelectConstructor;
	export = FilteringSelect;
}

We use the older module syntax to make it clear we are declaring an importable module, versus a name space. We have separate constructor and instance interfaces instead of classes and we use name merging above to make it "work". Any other exports/types of the module need to be on the constructor interface or accessed through the other global namespace of dojo and dijit.

@googol
Copy link

googol commented Sep 27, 2017

Hi.

I dropped the exports = Big in favor of export Big, to make the use of the library easier in an es6 codebase (I added that named export to big.js itself (pull request)). With the named export you'll need to either:

import * as BigJs from 'big.js';
const lookABigJs = new BigJs.Big(1);

or

import { Big } from 'big.js';
const lookABigJs = new Big(1);

That will fail at runtime however, if webpack actually bundles the AMD export, since the shape of the AMD and commonjs exports is different, which I didn't expect to be a problem.
I'd just disable the AMD importing from webpack myself. If I've understood correctly, you'll need to add a new loader like this:

    {
      test: /\.js$/,
      use: 'imports-loader?define=>false',
      include: /node_modules/,
    }

Documentation for the loader: https://webpack.js.org/loaders/imports-loader/#disable-amd
IMHO this should be the default behavior for webpack for anything in node_modules, but that's something for some other place and time.

To make this less painful, I suppose the best would be to match the exports in big.js itself (add the named export to AMD too).
I really hope we don't go back to exports = Big since that is very painful to use in an es6 codebase.
(This code: import * as Big from 'big.js'; const x = Big(3); doesn't work in all contexts and is against the module syntax spec.)

on 2.
The class syntax is very nice and I would have preferred using it for the typings, but there are a couple of features of big.js that cannot be modelled with it, namely calling the constructor without new to create a new Big value, and the quite odd feature of callign the constructor without new and without parameters to create a new copy of the constructor (with its own set of configuration variables). I don't think leaving those features out of the typings can be justified.

Rewriting as class won't help with 1. anyways. To a user both of these look approximately the same:

class Big {
}
exports = Big;
interface BigConstructor{
}
const Big: BigConstructor;
exports = Big;

I hope this helps. Let me know if I can help with this further

@RyanCavanaugh
Copy link
Member

Re this code:

import * as BigJs from 'big.js';
const lookABigJs = new BigJs(1);

See https://stackoverflow.com/questions/39415661/what-does-resolves-to-a-non-module-entity-and-cannot-be-imported-using-this . Working hard trying to make this work is counterproductive because it's not legal according to the ES6 spec.

In general when you have a library that you want to import and then immediately new, you should be writing the CommonJS form

import big = require('big.js');

These libraries will hopefully be moving their class export to a default export in the future so you can write

import big from 'big.js'

@RyanCavanaugh RyanCavanaugh added the Question An issue which isn't directly actionable in code label Sep 27, 2017
@johnnyreilly
Copy link
Author

johnnyreilly commented Sep 27, 2017

Hi @RyanCavanaugh / @googol and @kitsonk,

Thanks all for responding.

To take things in reverse order:

How can we model classes that are more than they seem? Are we limited to decomposed classes

I think it's clear that decomposed classes are the only game in town. Fine.

@kitsonk expresses the problem well when he says:

there is a different shape depending on which loader is being used and I am not sure that makes a huge amount of sense. The AMD shape is different than the CommonJS shape. While that is technically possible, that feels like that is an issue.

To be clear, I think it's "possible" in JavaScript but unless I've misunderstood it's not "possible" to model in TypeScript. Please do correct me if I misunderstood there. Thanks for sharing a proposed solution; indeed I can drop this in:

declare module 'big.js' {
    const Big: BigConstructor;
    export = Big;
}

Which allows us to do this:

import Big from 'big.js';
const lookABigJs = new Big(1);

However, whilst it gives with one hand it takes with the other. What I really want to do, and what @googol is doing it seems, is this:

import { Big } from 'big.js';
const lookABigJs = new Big(1);

The above definition amend torpedos the usage above. Also, since that's very much the desired usage I think that's a backward step. I feel you @googol when you say this:

I really hope we don't go back to exports = Big since that is very painful to use in an es6 codebase.
(This code: import * as Big from 'big.js'; const x = Big(3); doesn't work in all contexts and is against the module syntax spec.)

Our codebase is ES6 as well. However the only way you have it working at present is by killing AMD imports in webpack. I don't think this is good as AMD imports might be needed for some other package in the future. So I don't think this is an advisable course of action in the long term.

Something I've tried is amending big.js to make CommonJS the first branch in the export if statement like so:

    if (typeof module !== 'undefined' && module.exports) {
        module.exports = Big;
        module.exports.Big = Big;
    } else if (typeof define === 'function' && define.amd) {
        //AMD.
        define(function () {
            return Big;
        });

        //Browser.
    } else {
        global.Big = Big;
    }

I've tested this locally and it works. I'm up for sending a PR to the 'big.js' project and hopefully getting that into a future release. Is that the only route out of dodge?

But am I then right in thinking that this a limitation of TypeScript definition files as is? Where the different module format is different shapes (as here) then there is no way to cater for that in definition files? It sounds like "yes" which would be good information to have. I think it's almost certainly "yes" since TypeScript doesn't know how the file is being delivered (i.e. which module format served it up)

Oh and as an aside:

import big = require('big.js');

unfortunately does not inspire webpack into using the CommonJS import. (But upon reflection I guess that makes sense)

@johnnyreilly
Copy link
Author

Last night I raised a PR against big.js to fix the problems we were experiencing yesterday: MikeMcl/big.js#87

@googol made a better suggestion. The @MikeMcl implemented that and released v4.0 of big.js overnight.

@googol submitted another PR this morning which I merged: DefinitelyTyped/DefinitelyTyped#20096

v4.0 of the type definitions were published this morning. Yay open source! I might try and write this up as a blog post. Spread the knowledge and all that.

@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

4 participants