3.4.1 breaks backward compatibility for users of ReadonlyArray

[fkleuver]

When I tried to update aurelia to the latest TypeScript version, intellisense broke left right and center while the build still succeeds. It appears that ReadonlyArray<never> is emitted as readonly never[] in the type definitions, and the current version of VS Code doesn't understand that.

TypeScript Version: 3.4.1

Search Terms: 3.4.1, ReadonlyArray, readonly

Code

This is the source code:

export const PLATFORM = {
  global: {},
  emptyArray: Object.freeze([]),
  emptyObject: Object.freeze({}),
  noop: () => {},
};

Where the emptyArray resolves to ReadonlyArray<never>:

This is the generated type definition:

export declare const PLATFORM: {
    global: {};
    emptyArray: readonly never[];
    emptyObject: Readonly<{}>;
    noop: () => void;
};

And this is what code consuming that type definition will see:

Repro:

Not sure how to reproduce this on the typescript playground because it relies on cross-package references, so here is a minimal repro here:

git clone [email protected]:fkleuver/ts-readonly-issue.git
cd ts-readonly-issue
npm ci
npm run build

All will succeed. Now, open packages/runtime/src/index.ts and you'll see the problem:
The ReadonlyArray<any> falls back to any, and any properties after seem to disappear.

Expected behavior:
I would expect ReadonlyArray<never> to be emitted as ReadonlyArray<never> in the type definitions.

Actual behavior:

It is emitted as readonly never[].

This breaks backwards compatibility for anyone using ReadonlyArray and building with 3.4.1+, and will force all consumers to use this same version of TypeScript. Which includes VS Code at the moment :)

[ajafff]

I already reported that in #29435 (comment). This was also discussed in the release planning of v3.4 #30281:

• readonly type modifier for arrays/tuples
  • Merged, but...
  • Appears to be a large breaking change
    • breaks .d.ts consumption for older versions
    • issues with current consumers of displayed types (e.g. dts-lint) - how do you "expect" different displays of the same semantic type.
  • Needs investigation on workarounds.

But it seems there were no further actions to mitigate those breaks.

---

the current version of VS Code doesn't understand that.

You could use the workspace version of TypeScript instead of the version that comes bundled with VSCode.

I would expect ReadonlyArray to be emitted as ReadonlyArray in the type definitions.

That is actually the case if you declare something like var v: ReadonlyArray<never>;. However, if the type is inferred (var v: Readonly<Array<never>>;) it will be emitted with the new syntax.

I created a transformer to downlevel readonly array types in declaration files for backwards compatibility: https://github.com/ajafff/ts-transform-readonly-array

[fkleuver]

Thanks for this additional information. I was not able to find it between all the irrelevant search results showing up on various keywords.

• Appears to be a large breaking change
  • breaks .d.ts consumption for older versions

I think the TypeScript team is underestimating just how big of a breaking change this is. This will completely split the ecosystem into pre-3.4 and post-3.4 type definitions.

Since 3.4 (and a couple versions before that) also introduce breaking changes in projects themselves, and most of those project's maintainers likely don't have the time or resources to fix those issues, a significant portion of the ecosystem will not be able to make this update anytime soon.

This forces all libraries who happen to know about this, and care about their users, to stay on 3.3, or if they don't know about this, they will break everything downstream until someone reports an issue.

You could use the workspace version of TypeScript instead of the version that comes bundled with VSCode.

It doesn't work. As you can see in my repro example, I am using the vscode settings to explicitly point to the workspace version.

That is actually the case if you declare something like var v: ReadonlyArray;. However, if the type is inferred (var v: Readonly<Array>;) it will be emitted with the new syntax.

I don't understand the benefit of this. I understood this feature as being syntactic sugar - having a nice concise way to declare a ReadonlyArray in the source code. Why not do it the other way around and emit ReadonlyArray even if you declare readonly type[]?

Why the decision to make this breaking change about as breaking as it could possibly be, out of all the possible flavors that could have been picked?

And why do this in a minor version bump?

I created a transformer to downlevel readonly array types in declaration files for backwards compatibility:

This is great, thanks for that. This really ought to be a built-in opt-out with this patch, with an explicit flag to actually emit readonly type[].

I really like the later features of TypeScript. The team is doing a great job here.
For Aurelia vNext we are constantly utilizing the newest and fanciest features of TypeScript ever since 3.0 build system was introduced. Up until this point we've never had stuff break like this.

But this kind of versioning, in my opinion, is subpar. Surely, one should be able to upgrade a minor version without having to worry about breaking changes at all, certainly of this magniture?

@DanielRosenwasser would you guys please consider emitting readonly type[] as an explicit opt-out of compatibility mode, and in any other case emit ReadonlyArray<type>, for the sake of the community?

[Airblader]

And why do this in a minor version bump?

This is a bit off topic, but: TypeScript doesn't follow (or claim to follow) semantic versioning, so it's more of a "why not".

[fkleuver]

Right, I found this hidden away issue about semver and it seems that the major version is marketing, the minor version is breaking, and the patch is anything feature or bugfix related. Good to know.

This is still a pretty nasty one considering it affects all inferred types as you say, which includes the commonly used Object.freeze() as demonstrated in the repro

[EisenbergEffect]

@DanielRosenwasser When you are back in office, it would be great to get some eyes on this. I think this change has likely created a bigger issue than anticipated, or the effects were accidentally overlooked. I'd love to hear what can be done to make this better.

[weswigham]

For anyone reading and thinking that they're blocked by this, In any case where your declaration file output would be impacted by this, you can explicitly write the annotation including ReadonlyArray<whatever> at that location. This is not without workarounds for those which want backwards compatibility in their declaration files,
eg:

export const x = Object.freeze([1,2,3]);
// Emits `export const x: readonly number[]

export const y: ReadonlyArray<number> = Object.freeze([1,2,3]);
// Emits `export const x: ReadonlyArray<number> as written

We've gone thru the same process with a number of new type-syntax features that get used in declaration files, including import types, tuple types, conditional types... Pretty much every time we add a new form of type (this time we added readonly tuples by extended the syntax to cover arrays).

So, taking your example in the OP:

export const PLATFORM: { global: {}, emptyArray: ReadonlyArray<never>, emptyObject: Readonly<{}>, noop: () => {} } = {
  global: {},
  emptyArray: Object.freeze([]),
  emptyObject: Object.freeze({}),
  noop: () => {},
};

Canonically, this is how you deal with new syntax finding its way into declaration files - you simply explicitly override it with the (backwards compatible) syntax you want.

[fkleuver]

@weswigham Thank you for the clarification. I'll close this since there isn't really any action to be taken here.