Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build broken due to string enum member check failing #863

Closed
3 tasks done
aoberoi opened this issue Sep 12, 2019 · 3 comments · Fixed by #864
Closed
3 tasks done

Build broken due to string enum member check failing #863

aoberoi opened this issue Sep 12, 2019 · 3 comments · Fixed by #864
Labels
tests M-T: Testing work only

Comments

@aoberoi
Copy link
Contributor

aoberoi commented Sep 12, 2019

Description

All builds (for all open PRs) are currently broken because of a TypeScript build error in the @slack/rtm-api package:

src/RTMClient.ts:136:74 - error TS2345: Argument of type 'string' is not assignable to parameter of type 'UnrecoverableRTMStartError'.

136                       Object.values(UnrecoverableRTMStartError).includes(error.data.error)) {

I'm still gathering information to understand the cause, but I'll share what I know so far. The builds were passing on my local machine before I cleaned my working copy (by removing all node_modules directories and all package-lock.json files). After that, I got the same errors locally.

I also investigated the difference in how the tests are invoked on CI versus locally. Specifically, on CI the --no-ci --hoist flags are passed to lerna bootstrap. I can confirm that those flags have nothing to do with this issue. The --no-ci flag makes the CI environment work more like a local environment, and the --hoist flag moves common dependencies to the root node_modules directory. Since the errors occurred without either of those flags locally, its safe to conclude that the error is not related to them.

My next hunch is that it has to do with a later version of typescript being installed. The version that is installed across all packages locally when I reproduced the error is 3.6.3, while the package.json files specify a range of ^3.3.3333. It is possible that the semantics of a string enum changed in TypeScript releases 3.4, 3.5, or 3.6. That's what I'll be investigating next.

Requirements (place an x in each of the [ ])

  • I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've searched for any related issues and avoided creating a duplicate issue.
@aoberoi aoberoi added bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented tests M-T: Testing work only and removed bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented labels Sep 12, 2019
@aoberoi
Copy link
Contributor Author

aoberoi commented Sep 12, 2019

I can confirm that this issue does seem to be tied to the version of typescript.

  • changed packages/rtm-api/package.json's typescript devDep range to ~3.3.3333, which resolved to 3.3.4000 ➡️ works

  • changed packages/rtm-api/package.json's typescript devDep range to ~3.4.0, which resolved to 3.4.5 ➡️ works

  • changed packages/rtm-api/package.json's typescript devDep range to ~3.5.0, which resolved to 3.5.3 ➡️ works

  • changed packages/rtm-api/package.json's typescript devDep range to ~3.6.0, which resolved to 3.6.3 ➡️ does not work

So it seems there's some breaking change in TypeScript 3.6 that is responsible for this. However, after checking the list of breaking changes in that release, there doesn't seem to be a specific intentional change responsible for it.

I guess this means either its an unintentional breaking change (bug in TypeScript) or one that is much too subtle for me to relate directly back to the intentional ones.

Still investigating!

@aoberoi
Copy link
Contributor Author

aoberoi commented Sep 12, 2019

@aoberoi
Copy link
Contributor Author

aoberoi commented Sep 12, 2019

For now, I think it's prudent to apply a workaround and get builds back to working.

We should also follow along with the issue above to see if we can remove the ugliness of the workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests M-T: Testing work only
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant