Skip to content

Conversation

danrosenthal
Copy link

Below type error solved with d52c0a6

┌🤘-🐧danrosenthal@ 💻 Daniels-MacBook-Pro - 🧱 polaris-react on 🌵  typescript-3.5.1-upgrade •2 ✗
└🤘-> yyarn sewing-kit type-check
yarn run v1.13.0
$ /Users/danrosenthal/Projects/polaris-react/node_modules/.bin/sewing-kit type-check
[type-check] Checking tsconfig.json
src/components/AppProvider/utilities/createAppProviderContext/createAppProviderContext.ts:60:7 - error TS2322: Type 'ClientApplication<unknown> | undefined' is not assignable to type 'ClientApplication<{}> | undefined'.
  Type 'ClientApplication<unknown>' is not assignable to type 'ClientApplication<{}>'.
    Type 'unknown' is not assignable to type '{}'.

60       appBridge,
         ~~~~~~~~~

  src/components/AppProvider/types.ts:41:5
    41     appBridge?: ClientApplication<{}>;
           ~~~~~~~~~
    The expected type comes from property 'appBridge' which is declared here on type '{ intl: Intl; link: Link; stickyManager: StickyManager; scrollLockManager: ScrollLockManager; appBridge?: ClientApplication<{}> | undefined; }'


Found 1 error.

@BPScott BPScott temporarily deployed to polaris-react-pr-1650 June 7, 2019 12:48 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1650 June 7, 2019 12:49 Inactive
@danrosenthal danrosenthal changed the title Typescript 3.5.1 upgrade Upgrade Typescript to 3.5.1 Jun 7, 2019
@danrosenthal
Copy link
Author

Ping @Shopify/app-bridge

@danrosenthal
Copy link
Author

danrosenthal commented Jun 7, 2019

Any thoughts on what to do about the CI check failing with?

#!/bin/bash -eo pipefail
yarn run check:ci
yarn run v1.10.1
$ npm-run-all lint ts 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.5.0

YOUR TYPESCRIPT VERSION: 3.5.1

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

Sounds like it's safe to ignore, but can we silence these warnings so CI will pass? Looks like this version of TypeScript is supported in master and maybe the canary release of this package: https://github.com/typescript-eslint/typescript-eslint/blob/master/package.json#L81. Do we need to wait for another release of this dependency?

@BPScott
Copy link
Member

BPScott commented Jun 7, 2019

We'll need to either (in descending order of preference):

  • drop down to TS 3.4.5 for the moment (and hope that fixes Dan's "oh god the tsserver is giving making my laptop sound like it wants to take off" issue)
  • wait for either a new version of typescript-eslint/parser which supports 3.5.x - as you say they've got the fix in master we just waiting on the release, I'm reasonably confident they'll be reasonably quick about it
  • set the (warnOnUnsupportedTypeScriptVersion parser option)[https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/parser/README.md#configuration] to false to suppress that warning.

Can we bare to wait for a few days and see if typescript-eslint/parser gets a new release then if not,
If 3.4.5 fixes Dan's CPU usage issue then I think we should drop down to that for the moment as there's nothing else we're champing at the bit for in v3.5.0.


I really dislike that resolution trick as it feels really fragile but hey it seems to work and hopefully we'll get eslint-plugin-shopify using typescript-eslint/parser within the next few weeks so it can go away (hi @cartogram)

@danrosenthal
Copy link
Author

danrosenthal commented Jun 7, 2019

Can we bare to wait for a few days and see if typescript-eslint/parser gets a new release then if not,
If 3.4.5 fixes Dan's CPU usage issue then I think we should drop down to that for the moment as there's nothing else we're champing at the bit for in v3.5.0.

This feels like a good plan to me. I'll throw a WIP on this for now.

In the mean time I'll just be watching https://github.com/typescript-eslint/typescript-eslint waiting for a release.

@danrosenthal danrosenthal changed the title Upgrade Typescript to 3.5.1 [WIP][Waiting on dependency release] Upgrade Typescript to 3.5.1 Jun 7, 2019
@danrosenthal danrosenthal changed the title [WIP][Waiting on dependency release] Upgrade Typescript to 3.5.1 Upgrade Typescript to 3.5.1 Jun 10, 2019
@danrosenthal
Copy link
Author

Well that was a quick turnaround: https://github.com/typescript-eslint/typescript-eslint/releases/tag/v1.10.2

ping: @BPScott

Copy link
Member

@BPScott BPScott left a comment

Choose a reason for hiding this comment

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

Hurrah for patience!

@danrosenthal danrosenthal merged commit a3f6892 into master Jun 10, 2019
@danrosenthal danrosenthal deleted the typescript-3.5.1-upgrade branch June 10, 2019 16:38
@dleroux dleroux temporarily deployed to production June 11, 2019 14:46 Inactive
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.

3 participants