@diosmosis opened this Pull Request on December 27th 2021 Member

Description:

This PR is based off of #18446.

By default Vue sets the 'transpile only' option in the TypeScript compiler which only does a cursory check of typing related errors. This means the current code has a lot of typing errors (and also some bugs) that were never detected. It also means we can't output typing information for plugin UMDs, so type information for CoreHome would be lost when using CoreHome exports in another plugin. This PR fixes that. Note: Vue doesn't actually support doing a full compile natively (there's at least one open issue), so we workaround it.

(This is also why typescript errors in unit tests do not appear in vue:build.)

Changes:

  • In vue.config.js if in production mode, turn on full compilation. In development mode this is not done so development will still be fast (using full type checking takes a bit longer since tsc will do more work).
  • Output typings for plugin UMDs to the /path/to/matomo/<a class='mention' href='https://github.com/types'>@types</a> directory. These will be picked up by a change to tsconfig.json and typescript will be able to apply typing information to JS imported from other plugins.
  • Fixes for all typing errors. Some are superficial, but there are some actual bug fixes in here as well.
    Note: one "fix" is a workaround for vue's DeepReadonly type not being able to handle recursive interfaces (eg, our Widget data structure). Fixed by separating the part that recurses and casting to a sub-interface when needed.

Review

@diosmosis commented on January 10th 2022 Member

@sgiehl

As I'm not very experienced with typescript I can't really assess if all those changes would be needed or what they are good for.

You can see why a change was made doing the following:

  • revert the change locally
  • run vue:build $pluginName. tsc will fail with an error and that will tell you why the change was made.

This PR is just fixing typing errors that tsc complains about when you set it to actually use and generate typing information (by default vue does not enable this, so much of the typescript code that was written was incorrect). This PR would not be large or even necessary if this setting change was done in the beginning, but vue does not seem to mention that it doesn't do real type checking.

@sgiehl commented on January 10th 2022 Member

Yeah sure. Feel free to merge it then. If any regressions may pop up later we can fix them then as well. I don't think it's needed to test everything in detail again now

This Pull Request was closed on January 10th 2022
Powered by GitHub Issue Mirror