Skip to content

feat: add NonNullable builtin type to remove null from type union - #1875

Merged
dcodeIO merged 19 commits into
AssemblyScript:mainfrom
willemneal:feat/not-nullable
Jun 8, 2021
Merged

feat: add NonNullable builtin type to remove null from type union#1875
dcodeIO merged 19 commits into
AssemblyScript:mainfrom
willemneal:feat/not-nullable

Conversation

@willemneal

Copy link
Copy Markdown
Contributor

This adds the TS type mentioned by @MaxGraey and should fix #1874. Also did a little refactor for asserting single type argument.

Also I'm not sure why my change removed the element section in the wat files. I tried reverting the refactor and just having my single new function and it still generated the same wat files.

@willemneal willemneal changed the title feat: add NonNullable builtin type to remove null from T feat: add NonNullable builtin type to remove null from type union May 29, 2021
@MaxGraey

Copy link
Copy Markdown
Member

looks like you forget loacally update deps

Comment thread src/resolver.ts Outdated
Willem Wyndham and others added 2 commits May 29, 2021 11:18
@willemneal
willemneal requested a review from MaxGraey May 29, 2021 15:48
@jtenner

jtenner commented May 29, 2021

Copy link
Copy Markdown
Contributor

This will make aspect better! Thank you.

Comment thread tests/compiler/nonnullable.ts Outdated
@MaxGraey
MaxGraey requested a review from dcodeIO June 1, 2021 15:08
Comment thread tests/compiler/nonnullable.ts Outdated
@willemneal
willemneal requested a review from MaxGraey June 1, 2021 20:47
Comment thread tests/compiler/NonNullable.ts Outdated
@willemneal

Copy link
Copy Markdown
Contributor Author

@MaxGraey @dcodeIO anything else?

@willemneal
willemneal requested a review from MaxGraey June 7, 2021 13:35
Comment thread src/resolver.ts Outdated
Comment thread src/resolver.ts Outdated
Willem Wyndham and others added 2 commits June 7, 2021 09:50
Co-authored-by: Max Graey <maxgraey@gmail.com>
@willemneal
willemneal requested a review from MaxGraey June 7, 2021 14:30
Comment thread src/resolver.ts Outdated
Comment thread src/resolver.ts Outdated
@willemneal

Copy link
Copy Markdown
Contributor Author

Okay went with your suggestion. Still think it's a bit of an over optimization, but I want to get this merged.

Comment thread src/common.ts Outdated
@willemneal
willemneal requested a review from dcodeIO June 7, 2021 17:10
@dcodeIO

dcodeIO commented Jun 7, 2021

Copy link
Copy Markdown
Member

The package-lock changes seem unrelated, can you unstage them before merging? :)

Comment thread tests/compiler/nullable.ts
@dcodeIO

dcodeIO commented Jun 7, 2021

Copy link
Copy Markdown
Member

Also, there seems to be some inconsistency with line endings at the end of new files. While probably not super important, it would be great to always have an \n at the end I think :)

@willemneal

Copy link
Copy Markdown
Contributor Author

@dcodeIO I added a new line to the end of the test .ts file, but all of the json files seem to all not have a \n at the end of the file.

Comment thread tests/compiler/nonnullable.json Outdated
@willemneal
willemneal requested a review from dcodeIO June 7, 2021 18:30
Comment thread std/assembly/index.d.ts
/** A special type evaluated to the return type of T if T is a callable function. */
declare type returnof<T extends (...args: any) => any> = ReturnType<T>;
/** A special type that excludes null and undefined from T. */
declare type NonNullable<T> = T extends null | undefined ? never : T;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, how does this work?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, toyed around with it a little and it works, I just don't understand how

@willemneal willemneal Jun 8, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a conditional type. NonNullable<null> becomes the never type which would cause it to fail to typecheck. We don't have this issue because it's not possible to type something as null, see here

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My guess is that this somehow decomposes string | null (which extends null I guess?) to string and null, with the null turned into never, so that the outcome would be string | never with the never being dropped or something.

@willemneal
willemneal requested a review from dcodeIO June 8, 2021 17:32
@willemneal

Copy link
Copy Markdown
Contributor Author

@dcodeIO I made a script that added new lines to source files in the tests folder that were missing them.

@dcodeIO

dcodeIO commented Jun 8, 2021

Copy link
Copy Markdown
Member

Would have preferred a separate PR as noted earlier, but now that you already did it I guess we can just do it here

@dcodeIO
dcodeIO merged commit 42883cb into AssemblyScript:main Jun 8, 2021
@dcodeIO

dcodeIO commented Jun 8, 2021

Copy link
Copy Markdown
Member

Thanks!

@willemneal

Copy link
Copy Markdown
Contributor Author

Got confused with your question. But glad it's merged!

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

Successfully merging this pull request may close these issues.

[Feature Request] Built in to make a type non-nullable

4 participants