Upgrade typescript to 3.7.2 #2237

danrosenthal · 2019-10-03T20:36:20Z

github-actions · 2019-10-03T20:38:49Z

💦 Potential splash zone of changes introduced to src/**/*.tsx in this pull request:

Files modified	10
Files potentially affected	1

Details

All files potentially affected (total: 1)

/src/components/AppProvider/AppProvider.tsx

📄 UNRELEASED.md (total: 0)

Files potentially affected (total: 0)

📄 package.json (total: 0)

Files potentially affected (total: 0)

🧩 src/components/ContextualSaveBar/ContextualSaveBar.tsx (total: 0)

Files potentially affected (total: 0)

🧩 src/components/DropZone/tests/DropZone.test.tsx (total: 0)

Files potentially affected (total: 0)

🧩 src/components/RangeSlider/RangeSlider.tsx (total: 0)

Files potentially affected (total: 0)

🧩 src/components/RangeSlider/components/DualThumb/tests/DualThumb.test.tsx (total: 0)

Files potentially affected (total: 0)

🧩 src/components/ThemeProvider/ThemeProvider.tsx (total: 1)

Files potentially affected (total: 1)

/src/components/AppProvider/AppProvider.tsx

🧩 src/components/Toast/Toast.tsx (total: 0)

Files potentially affected (total: 0)

🧩 src/utilities/tests/is-element-in-viewport.test.ts (total: 0)

Files potentially affected (total: 0)

📄 yarn.lock (total: 0)

Files potentially affected (total: 0)

This comment automatically updates as changes are made to this pull request.
Feedback, troubleshooting: open an issue or reach out on Slack in #polaris-tooling.

danrosenthal · 2019-10-03T20:44:58Z

😢

#!/bin/bash -eo pipefail
yarn run check:ci
yarn run v1.10.1
$ npm-run-all lint type-check test:ci
$ sewing-kit lint

=============

WARNING: You are currently running a version of TypeScript which is not officially supported by typescript-estree.

You may find that it works just fine, or you may not.

SUPPORTED TYPESCRIPT VERSIONS: >=3.2.1 <3.6.0

YOUR TYPESCRIPT VERSION: 3.6.3

Please only submit bug reports when using the officially supported version.

=============


error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
ERROR: "lint" exited with 1.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Exited with code 1

danrosenthal · 2019-10-03T20:52:15Z

I think past me could be responsible for this issue (#1650 ). What do you think @BPScott?

Specifically this line: https://github.com/Shopify/polaris-react/pull/1650/files#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R167

BPScott · 2019-10-03T21:28:18Z

Yeah that looks likely, typescript-eslint-parser got deprecated and you need to move over to typescript-eslint/parser to support new versions of TS. We've still not got a fix for that into eslint-plugin-shopify yet so we still need that resolution, but we might be able to bump the version that it points at

BPScott · 2019-10-24T19:07:54Z

Yay a new SK with a new eslint-plugin-shopify has been released that removes the need for that resolutions dance. I'll see about updating in the new week or so

BPScott · 2019-10-26T01:10:15Z

#2369 is the PR for the SK update, but alas we're blocked on that because thanks annoyances with the prettier-vscode version (we've updated to eslint 6 but they haven't and that causes problems)

BPScott

I took the liberty of rebasing this and when i saw 3.6.3 worked flawlessly I felt adventurous and tried to update to 3.7, which required a few small changes

BPScott · 2019-11-09T01:06:36Z

src/components/ContextualSaveBar/ContextualSaveBar.tsx


 // The script in the styleguide that generates the Props Explorer data expects
 // a component's props to be found in the Props interface. This silly workaround
 // ensures that the Props Explorer table is generated correctly, instead of
-// crashing if we write `ContextualSaveBar extends React.Component<ContextualSaveBarProps>`
-export interface ContextualSaveBarProps extends ContextualSaveBarProps {}


TS 3.7 stopped you having a local type with the same name as an import https://devblogs.microsoft.com/typescript/announcing-typescript-3-7/#local-and-imported-type-declarations-now-conflict

The renaming dance is stupid but the props explorer doesnt find the types if you don't define the type in this file

I thumbs upped your comment but I do not thumbs up anything that's going on here

BPScott · 2019-11-09T01:07:51Z

src/components/DropZone/tests/DropZone.test.tsx

@@ -469,6 +469,9 @@ function setBoundingClientRect(size: keyof typeof widths) {
        left: 0,
        bottom: 0,
        right: 0,
+        x: 0,


Updates to the type that Element.getBoundingClientRect() returns added these new values so we need to add them to the mock (this crops up a few times) https://devblogs.microsoft.com/typescript/announcing-typescript-3-7/#dom-changes

BPScott · 2019-11-09T01:09:46Z

src/components/ThemeProvider/ThemeProvider.tsx

  // Otherwise, setting a style property to `undefined` does not remove it from the DOM.
-  const backgroundColor = customProperties['--p-surface-background'] || null;
-  const color = customProperties['--p-text-on-surface'] || null;
+  const backgroundColor = customProperties['--p-surface-background'] || '';


backgroundColor is now not nullable. color can be nullable but i figured we might as well make this consistent as it seems to behave the same in chrome for me. @tmlayton can you confirm the intended behaviour still works for you?

If the 🎩 looks good I’m fine with it. Try toggling global theming on and off and make sure it works.

BPScott · 2019-11-09T01:22:14Z

While 3.7.x is super shiny, we might need update to babel 7.7.x and poke at our babel config for full support of stuff like optional chaining and nullish coalescing as the storybook build is done using babel rather than tsc, but that's a task for later.

amrocha

We should do something about that whole styleguide props explorer script thing huh

amrocha · 2019-11-18T20:44:58Z

src/components/ContextualSaveBar/ContextualSaveBar.tsx


 // The script in the styleguide that generates the Props Explorer data expects
 // a component's props to be found in the Props interface. This silly workaround
 // ensures that the Props Explorer table is generated correctly, instead of
-// crashing if we write `ContextualSaveBar extends React.Component<ContextualSaveBarProps>`
-export interface ContextualSaveBarProps extends ContextualSaveBarProps {}


I thumbs upped your comment but I do not thumbs up anything that's going on here

amrocha · 2019-11-18T20:53:02Z

src/components/Toast/Toast.tsx

@@ -2,16 +2,16 @@ import React, {useRef} from 'react';
 import {Toast as AppBridgeToast} from '@shopify/app-bridge/actions';

 import {DEFAULT_TOAST_DURATION} from '../Frame';
-import {ToastProps, useFrame} from '../../utilities/frame';
+import {ToastProps as ToastProps1, useFrame} from '../../utilities/frame';


thanks I hate it

This reverts commit 9085064.

Revert "Upgrade typescript to 3.7.2 (#2237)"

This reverts commit f7bf6cf.

* Revert "Revert "Upgrade typescript to 3.7.2 (#2237)"" This reverts commit f7bf6cf. * Use class methods instead of getters of undefined properties If you have an uninitialized property that then gets referenced (like by having a getter for it) then TS emits declarations that break in consuming projects. Convert a bunch of cases where we created getters for non-existant properties to use boring old method functions. * Update changelog

danrosenthal force-pushed the upgrade-typescript branch from 8b02c24 to 5a91b35 Compare October 3, 2019 20:37

danrosenthal changed the title ~~upgrade typescript to 3.6.3~~ [WIP] upgrade typescript to 3.6.3 Oct 3, 2019

BPScott force-pushed the upgrade-typescript branch 2 times, most recently from c9cd99e to 892a15b Compare November 9, 2019 01:04

BPScott reviewed Nov 9, 2019

View reviewed changes

BPScott changed the title ~~[WIP] upgrade typescript to 3.6.3~~ [WIP] upgrade typescript to 3.7.2 Nov 9, 2019

upgrade typescript

d6024bb

BPScott force-pushed the upgrade-typescript branch from 892a15b to 5c012da Compare November 18, 2019 18:40

BPScott changed the title ~~[WIP] upgrade typescript to 3.7.2~~ Upgrade typescript to 3.7.2 Nov 18, 2019

Typescript 3.7

4d4c4d7

BPScott force-pushed the upgrade-typescript branch from 5c012da to 4d4c4d7 Compare November 18, 2019 19:06

amrocha approved these changes Nov 18, 2019

View reviewed changes

BPScott merged commit 9085064 into master Nov 18, 2019

BPScott deleted the upgrade-typescript branch November 18, 2019 21:01

dleroux added a commit that referenced this pull request Dec 4, 2019

Revert "Upgrade typescript to 3.7.2 (#2237)"

f7bf6cf

This reverts commit 9085064.

dleroux mentioned this pull request Dec 4, 2019

Revert "Upgrade typescript to 3.7.2 (#2237)" #2501

Merged

6 tasks

dleroux added a commit that referenced this pull request Dec 4, 2019

Merge pull request #2501 from Shopify/revert-ts-update

c2a3275

Revert "Upgrade typescript to 3.7.2 (#2237)"

BPScott added a commit that referenced this pull request Dec 13, 2019

Revert "Revert "Upgrade typescript to 3.7.2 (#2237)""

460eb05

This reverts commit f7bf6cf.

BPScott mentioned this pull request Dec 13, 2019

TypeScript 3.7 (take 2) #2549

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade typescript to 3.7.2 #2237

Upgrade typescript to 3.7.2 #2237

danrosenthal commented Oct 3, 2019

github-actions bot commented Oct 3, 2019 •

edited

Loading

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 1)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

danrosenthal commented Oct 3, 2019 •

edited

Loading

danrosenthal commented Oct 3, 2019 •

edited

Loading

BPScott commented Oct 3, 2019 •

edited

Loading

BPScott commented Oct 24, 2019

BPScott commented Oct 26, 2019

BPScott left a comment

BPScott Nov 9, 2019 •

edited

Loading

amrocha Nov 18, 2019

BPScott Nov 9, 2019

BPScott Nov 9, 2019

tmlayton Nov 13, 2019

BPScott commented Nov 9, 2019 •

edited

Loading

amrocha left a comment

amrocha Nov 18, 2019

amrocha Nov 18, 2019

Upgrade typescript to 3.7.2 #2237

Upgrade typescript to 3.7.2 #2237

Conversation

danrosenthal commented Oct 3, 2019

github-actions bot commented Oct 3, 2019 • edited Loading

Details

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 1)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

danrosenthal commented Oct 3, 2019 • edited Loading

danrosenthal commented Oct 3, 2019 • edited Loading

BPScott commented Oct 3, 2019 • edited Loading

BPScott commented Oct 24, 2019

BPScott commented Oct 26, 2019

BPScott left a comment

Choose a reason for hiding this comment

BPScott Nov 9, 2019 • edited Loading

Choose a reason for hiding this comment

amrocha Nov 18, 2019

Choose a reason for hiding this comment

BPScott Nov 9, 2019

Choose a reason for hiding this comment

BPScott Nov 9, 2019

Choose a reason for hiding this comment

tmlayton Nov 13, 2019

Choose a reason for hiding this comment

BPScott commented Nov 9, 2019 • edited Loading

amrocha left a comment

Choose a reason for hiding this comment

amrocha Nov 18, 2019

Choose a reason for hiding this comment

amrocha Nov 18, 2019

Choose a reason for hiding this comment

github-actions bot commented Oct 3, 2019 •

edited

Loading

danrosenthal commented Oct 3, 2019 •

edited

Loading

danrosenthal commented Oct 3, 2019 •

edited

Loading

BPScott commented Oct 3, 2019 •

edited

Loading

BPScott Nov 9, 2019 •

edited

Loading

BPScott commented Nov 9, 2019 •

edited

Loading