TypeScript 2.8 — initial support & updates #14

fbartho · 2018-04-03T07:31:40Z

A quick stab at importing some of the new best ways to implement these helper functions using TypeScript 2.8 features.

Removes discouraged Diff<…> helper from README and document suggestion of using Exclude
Removes now-public NonNullable implementation from README, and document that it's now globally available
Update implementations of Omit and Overwrite based on community & MS recommendations

Unfortunately, to test this (today), you have to manually patch definitelytyped-header-parser in your node_modules directory, as it doesn't yet support TypeScript 2.8 (via a constant) and because of this dtslint can't test the code. If you do patch that to allow TypeScript 2.8, then the tests pass.

Fixes #13

fbartho · 2018-04-03T07:44:09Z

As documented above, the tests is expected to fail in CI until definitelytyped-header-parser updates via: microsoft/definitelytyped-header-parser#12 which can then permit dtslint to update via microsoft/dtslint#113

pelotom · 2018-04-03T18:42:33Z

types/index.d.ts

- * @see https://github.com/Microsoft/TypeScript/issues/12215#issuecomment-307871458
+ * TypeScript Documentation suggests that as of 2.8 you should use the built-in "Exclude" instead.
+ *
+ * @see https://github.com/Microsoft/TypeScript-Handbook/blame/master/pages/release%20notes/TypeScript%202.8.md#L245


Let's either alias Diff to Exclude or just take it out; I think I'd be fine with the latter and bumping the major version since we're already making the breaking change of requiring TypeScript 2.8.

Currently making it an Alias, but happy to delete it instead, please advise!

Let's delete.

pelotom · 2018-04-03T18:43:57Z

types/index.d.ts

- *
- * @see https://github.com/Microsoft/TypeScript/issues/15012#issuecomment-346499713
+ * Deprecated: available in TypeScript Core as of 2.8
+ * @see https://github.com/Microsoft/TypeScript-Handbook/blame/master/pages/release%20notes/TypeScript%202.8.md#L203


Let's just take it out.

pelotom · 2018-04-03T18:50:08Z

types/index.d.ts

+export type Omit<T, K extends keyof T> = Pick<T, Exclude<keyof T, K>>;

 /**
 * Find the overlapping variants between two string unions.


Can we replace Overlap with Extract?

I'm not familiar with Overlap or the internals of Extract yet, happy to do whatever you suggest!

/** * Extract from T those types that are assignable to U */ type Extract<T, U> = T extends U ? T : never;

Diff and Overlap are dual, as Exclude and Extract are dual.

Cool! So what's the specific action you would like here? Sounds like the answer to your first question is "Yes", but do you want me to just delete the Overlap implementation from type-zoo?

Yes, I think Diff and Overlap have been superseded by Exclude and Extract respectively.

So let's delete both Diff and Overlap.

pelotom · 2018-04-03T18:52:44Z

types/index.d.ts

+ * @see https://github.com/Microsoft/TypeScript/issues/12215#issuecomment-377692897
 */
-export type Overwrite<T, U> = Omit<T, Overlap<keyof T, keyof U>> & U;
+export type Overwrite<T, U> = {


I don't see why this one needs to change except to use Extract instead of Overlap:

export type Overwrite<T, U> = Omit<T, Extract<keyof T, keyof U>> & U;

fbartho · 2018-04-03T19:05:25Z

Initially actionable changes addressed, happy to make more!

pelotom · 2018-04-03T19:17:43Z

Thanks, this is looking good on paper (although I haven't tried using it). Thanks also for tackling the dtslint issue, hopefully that's fixed soon and we can get this merged!

fbartho · 2018-04-03T19:48:08Z

If you want to test it early, the patch is in node_modules/definitelytyped-header-parser/index.js

-     TypeScriptVersion.all = ["2.0", "2.1", "2.2", "2.3", "2.4", "2.5", "2.6"];
+    TypeScriptVersion.all = ["2.0", "2.1", "2.2", "2.3", "2.4", "2.5", "2.6", "2.7", "2.8"];
// And later down
        "ts2.6",
+        "ts2.7",
+        "ts2.8",
        "latest",

fbartho · 2018-04-23T18:21:58Z

@pelotom Woooo! It's working!

pelotom · 2018-04-23T20:23:24Z

types/index.d.ts

 * @see https://github.com/Microsoft/TypeScript/issues/12215#issuecomment-307871458
 */
-export type Overwrite<T, U> = Omit<T, Overlap<keyof T, keyof U>> & U;
+export type Overwrite<T, U> = Omit<T, Extract<keyof T, keyof U>> & U;


Come to think of it, this could be (and always could have been) just

export type Overwrite<T, U> = Omit<T, keyof T & keyof U> & U;

There was never any need for the old Overlap, or am I missing something?

Haha, I don't know -- I didn't do TypeScript when that existed, does that pass the old tests? -- Do you want it changed to this?

I've never personally used the Overwrite operator, but your solution looks totally like an obvious implementation. I would probably have implemented it that way if I needed that operation.

Actually, looking at my code, I have exactly that pattern, but I don't do the keyof T & keyof U and just target the Omit directly at the keys I want to junk & replace.

Yeah, thinking it through I'm pretty sure Extract<keyof T, keyof U> is equivalent in all cases to keyof T & keyof U, so let's make that change.

pelotom

Looks great, thanks!

TypeScript 2.8 — initial support & updates

dd32b5c

pelotom requested changes Apr 3, 2018

View reviewed changes

fbartho added 3 commits April 3, 2018 12:02

PR Feedback - delete custom NonNullable now that public

803516b

PR Feedback: Simplify Overwrite Impl.

0970ff4

PR Feedback: make Diff an alias to Extract

8ff4824

PR Feedback: Delete Diff & Overlap as now provided by Exclude & Extract

229ea30

This was referenced Apr 11, 2018

Support TypeScript 2.8 in the header language version parser microsoft/definitelytyped-header-parser#12

Merged

Feature Request: Support for TypeScript 2.8 microsoft/dtslint#113

Closed

Update to new dtslint! — Tests pass!

034e1b7

pelotom requested changes Apr 23, 2018

View reviewed changes

PR Feedback: Simplify Overwrite

e0c4955

pelotom approved these changes Apr 23, 2018

View reviewed changes

pelotom merged commit 1399936 into pelotom:master Apr 23, 2018

TypeScript 2.8 — initial support & updates #14

TypeScript 2.8 — initial support & updates #14

Uh oh!

Conversation

fbartho commented Apr 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fbartho commented Apr 3, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fbartho commented Apr 3, 2018

Uh oh!

pelotom commented Apr 3, 2018

Uh oh!

fbartho commented Apr 3, 2018

Uh oh!

fbartho commented Apr 23, 2018

Uh oh!

pelotom Apr 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pelotom left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fbartho commented Apr 3, 2018 •

edited

Loading

pelotom Apr 23, 2018 •

edited

Loading