Skip to content
This repository has been archived by the owner on Oct 1, 2024. It is now read-only.

Update typescript to v4.3.5 #1997

Merged
merged 6 commits into from
Aug 12, 2021
Merged

Update typescript to v4.3.5 #1997

merged 6 commits into from
Aug 12, 2021

Conversation

BPScott
Copy link
Member

@BPScott BPScott commented Aug 12, 2021

Description

Keeping up to date. We get some perf improvements with incremental builds, and we can stop emitting and publishing typescript generated js files while keeping incremental build support, which leads to smaller package sizes as we're sending less cruft to npm.

You probably want to review this commit by commit, as most of the files changed are the changelogs which were added in the 3rd commit.

Type of change

  • Everything: patch.

@BPScott BPScott requested a review from a team August 12, 2021 00:45
interface FormatDateOptions extends Intl.DateTimeFormatOptions {
hourCycle?: string;
hourCycle?: 'h11' | 'h12' | 'h23' | 'h24';
Copy link
Member Author

@BPScott BPScott Aug 12, 2021

Choose a reason for hiding this comment

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

This is a stricter type, but the spec says that any value other than these will throw a runtime error so nobody would be calling this code with values other than these anyway.

Because of that I'm pretty happy to say that this is a bugfix rather than a breaking change.

See explosions yourself by running this:

console.log(new Intl.DateTimeFormat('en-US', { timeStyle: 'long', hourCycle:'trash' }).format())

@@ -140,14 +140,14 @@ export default class FormState<

// eslint-disable-next-line @shopify/react-prefer-private-members
public validateForm() {
return new Promise((resolve) => {
return new Promise<void>((resolve) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

TS now warns if you don't return anything from a promise as it suspects that might be a bug. If that's what you absolutely wanted to do then this is how you say "yes Typescript I really wanted to return nothing from this promise'

@@ -26,7 +26,7 @@ export const dateStyle = {
hour: '2-digit',
minute: '2-digit',
},
};
} as const;
Copy link
Member Author

Choose a reason for hiding this comment

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

Intl.DateTimeFormat now specifies the various options as a union of exact strings ('short' | 'long' etc) rather than any old strings so make this const so we know the values are those exact strings


### Changed

- `formatDate`'s options object now expects its `hourCycle` value to be `'h11', 'h12', 'h23' or 'h24' rather than an arbitary string. Any other value would have caused an error at runtime, this helps us catch that error at compile time. [[#1997](https://github.com/Shopify/quilt/pull/1997)]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- `formatDate`'s options object now expects its `hourCycle` value to be `'h11', 'h12', 'h23' or 'h24' rather than an arbitary string. Any other value would have caused an error at runtime, this helps us catch that error at compile time. [[#1997](https://github.com/Shopify/quilt/pull/1997)]
- `formatDate`'s options object now expects its `hourCycle` value to be `h11`, `h12`, `h23` or `h24` rather than an arbitary string. Any other value would have caused an error at runtime, this helps us catch that error at compile time. [[#1997](https://github.com/Shopify/quilt/pull/1997)]

### Changed

- `formatDate`'s options object now expects its `hourCycle` value to be `'h11', 'h12', 'h23' or 'h24' rather than an arbitary string. Any other value would have caused an error at runtime, this helps us catch that error at compile time. [[#1997](https://github.com/Shopify/quilt/pull/1997)]
- Updated bulid tooling, types are now compiled with TypeScript 4.3. [[#1997](https://github.com/Shopify/quilt/pull/1997)]
Copy link
Member

Choose a reason for hiding this comment

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

bulid => build throughout

Copy link
Member

@GoodForOneFare GoodForOneFare left a comment

Choose a reason for hiding this comment

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

👍 Banal nits aside, looks good.

@BPScott BPScott merged commit 14ab628 into main Aug 12, 2021
@BPScott BPScott deleted the ts4.3 branch August 12, 2021 17:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants