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

'readonly-array' regression for array literals in 5.0.0 #104

Closed
ulrichb opened this issue Dec 3, 2018 · 7 comments
Closed

'readonly-array' regression for array literals in 5.0.0 #104

ulrichb opened this issue Dec 3, 2018 · 7 comments

Comments

@ulrichb
Copy link

ulrichb commented Dec 3, 2018

Given the following mutable types SomeArray/SomeType, tslint-immutable 5.0.0 emits readonly-array also in the assignment sites (with array literals), but only in the SomeType (with an object literal) scenario. See the comments in my code snippet.

tslint-immutable 4.9.1 didn't do this which was correct IMO (because SomeType is declared as mutable, which is even library code in my real case).

type SomeArray = Array<string>; // tslint:disable-line:readonly-array

interface SomeType {
    readonly array: Array<string>; // tslint:disable-line:readonly-array
    readonly nested: {
        readonly array: Array<string>; // tslint:disable-line:readonly-array
    };
}

//

// no error in tslint-immutable 4.9.1/5.0.0
const arr: SomeArray = [""];

const o: SomeType = {
    // no error in tslint-immutable 4.9.1; but emits 'readonly-array' in 5.0.0:
    array: [""],

    nested: {
        // no error in tslint-immutable 4.9.1; but emits 'readonly-array' in 5.0.0:
        array: [""],
    },
};
@astorije
Copy link

We also find this to be a problem on types defined with ReadonlyArray.

If your interface is defined as such:

interface SomeType {
    readonly array: ReadonlyArray<string>;
    readonly nested: {
        readonly array: ReadonlyArray<string>;
    };
}

Then tslint-immutable 5.0.0 will now complain when passed literals such as array: [""]. Every array needs to be explicitly typed, even though the compiler gets the inference right on array literals.

@jonaskello
Copy link
Owner

@ulrichb @astorije I think this should be fixed in 5.0.1. Let me know if there are still regression for any cases.

@ulrichb
Copy link
Author

ulrichb commented Dec 15, 2018

@jonaskello Great. Many thanks!

@astorije
Copy link

@jonaskello, I haven't gotten the chance to test yet, but v5.0.1...master says that the fix was merged after 5.0.1 was released, is that incorrect?

@jonaskello
Copy link
Owner

@astorije Yes, you are correct. Seems I made the release while I was on the fix branch :-/. Also the update of the changelog was done on the wrong branch but I've fixed that now.

Although the tag is on the wrong commit I think the published code in 5.0.1 actually contains the fix. I did a npm install of it and it seems to be present in the js code that is installed. Let me know if it doesn't work.

@jonaskello jonaskello reopened this Dec 18, 2018
@phil-lgr
Copy link

phil-lgr commented Jan 4, 2019

I just upgraded to v5.0.1 works super now, thanks!

@jonaskello
Copy link
Owner

@phil-lgr Thanks for testing :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants