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

[WIP] Upgrade to babel@7 and support TypeScript #3292

Closed
wants to merge 4 commits into from

Conversation

turt2live
Copy link
Member

@turt2live turt2live commented Aug 8, 2019

Fixes element-hq/element-web#10524
Review with element-hq/element-web#10526
Review with matrix-org/matrix-js-sdk#1012

Upgrading to babel 7 gets a lot of the compilation stuff for free. The major changes we care about are:

  • The packages are now namespaced (hence all the package changes)
  • We use the env preset instead of the long-since deprecated es20XX packages.
  • Added --verbose to maintain log output from the previous version.

This also meant fixing ScalarAuthClient: we can no longer mix module.exports and import in the same file.

The SdkConfig was converted to TypeScript to prove this works (it does).

This specifically does not remove flow support because that's non-trivial. Instead, we just support it and call ourselves bad people for doing it. Source code that uses flow would be the first thing I'd recommend the team replace with TypeScript, time permitting.


TODO:

  • TypeScript basics
  • Babel upgrade
  • Fix i18n gen for TypeScript files
  • Verify sourcemaps are a thing that moderately work
  • Fix tests (if broken - they probably are)
  • Fix linting (if broken - probably is)

turt2live added a commit to element-hq/element-web that referenced this pull request Aug 8, 2019
See matrix-org/matrix-react-sdk#3292

Like react-sdk#3292, this fixes the couple source files which mix imports and module exports (they can't be mixed anymore).
turt2live added a commit to element-hq/element-web that referenced this pull request Aug 8, 2019
See matrix-org/matrix-react-sdk#3292

Like react-sdk#3292, this fixes the couple source files which mix imports and module exports (they can't be mixed anymore).
For element-hq/element-web#10524

Upgrading to babel 7 gets a lot of the compilation stuff for free. The major changes we care about are:
* The packages are now namespaced (hence all the package changes)
* We use the `env` preset instead of the long-since deprecated es20XX packages. 
* Added `--verbose` to maintain log output from the previous version.

This also meant fixing `ScalarAuthClient`: we can no longer mix `module.exports` and `import` in the same file. 

The SdkConfig was converted to TypeScript to prove this works (it does).
The update isn't super required, but it does help fix some of the module problems. We have to export ourselves as commonjs to make the tests happy, and empirically riot-web's webpack is also happy with this. 

The changes to the karma preprocessors is to ensure the app bootstraps correctly.
This makes the app happy.
turt2live added a commit to matrix-org/matrix-js-sdk that referenced this pull request Aug 9, 2019
@turt2live turt2live changed the title Upgrade to babel@7 and support TypeScript [WIP] Upgrade to babel@7 and support TypeScript Aug 9, 2019
@turt2live
Copy link
Member Author

turt2live commented Aug 9, 2019

The tests here and on the js-sdk have a fun problem where the Bluebird promises are not being converted to generators correctly, so it is angry. Using native promises does not work either (it breaks even worse).

Something is going strange with how the tests babel themselves, I think. The compiled code (the app) works fine but it's obviously a blocker if the tests can't run.

The error is:

TypeError: A value [object Object] was yielded that could not be treated as a promise

Resources:

However, we don't appear to be doing this either in tree or in the compiled output, so I'm not sure what's wrong. Nobody else has experienced this on the internet as far as I can tell.

The recommendation for using this generator plugin comes from chpio/babel-plugin-transform-async-to-bluebird#4 which appears to do the same thing (having looked at the diffs published by babel).

I don't really know where to proceed from here, and have given myself a maximum of 1 day to try and fix it. It's now the end of that day :(

@turt2live
Copy link
Member Author

A bit of progress on the tests: It looks like the compiled library code (at least for the js-sdk) is using promises that aren't compatible with Bluebird. Wrapping awaits like await Promise.resolve(theFunction()) works, but is obviously not great considering the amount of async/await code we have.

@turt2live
Copy link
Member Author

This has largely been replaced by other PRs

@turt2live turt2live closed this Jan 9, 2020
@turt2live turt2live deleted the travis/typescript branch January 9, 2020 21:06
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.

Use TypeScript
1 participant