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

TypeScript and Vue 3 #4559

Merged
merged 161 commits into from
Jun 19, 2022
Merged

TypeScript and Vue 3 #4559

merged 161 commits into from
Jun 19, 2022

Conversation

MaxLeiter
Copy link
Member

@MaxLeiter MaxLeiter commented May 2, 2022

This is a wide reaching PR, so to summarize my changes:

  • four tsconfigs:
    • ./tsconfig.base.json: shared values the other three extends
    • ./tsconfig.json: for the webpack.config.ts
    • ./src/tsconfig.json: server stuff
    • ./client/tsconfig.json: client stuff
  • three build commands in package.json now:
    • yarn build:client: runs webpack
    • yarn build:server: runs tsc to compile the server typescript files to src/dist
    • yarn build: runs yarn build:*
  • yarn dev now evokes ts-node to interpret the server side TS on the fly.
  • yarn start will run the compiled server (in src/dist)
  • dep changes:
    • added cross-env: sets env vars in package.json scripts for multiple platforms (this can probably be removed in the future)
    • added typings for packages without support for them
    • added necessary babel plugins and removed obsolete ones
    • added nyc typescript config (@istanbuljs/nyc-config-typescript)
    • added eslint-define-config, eslint-define-config
    • added ts-loader, ts-node, ts-sinon,
    • added tsconfig-paths, tsconfig-paths-webpack-plugin, which allow webpack/ts files to use path aliases (i.e. @components/)
    • updated vuex to 4.0 (was 3.6.2)
    • replaced vue-draggable with a hand-rolled Vue component using SortableJS (which vue-draggable uses under the hood)
  • I replaced most imports with requires but a few are left if they had advanced usage, like for loading client-side commands.

I've also moved us to Vue 3 for proper TypeScript support. We now use the composition API in our components

Sorry for the commit mess, I'll squash before merging. I used ts-migrate to try and automate some things but it really just renamed the files for me and made a bunch of commits while doing so.

@MaxLeiter MaxLeiter added the Status: WIP Work in progress or feature not fully fleshed out yet label May 2, 2022
@nemchik nemchik mentioned this pull request May 3, 2022
@MaxLeiter MaxLeiter marked this pull request as ready for review May 5, 2022 05:34
@MaxLeiter MaxLeiter added the Meta: Internal This is an internal codebase change (testing, linting, etc.). label May 5, 2022
Copy link
Member

@itsjohncs itsjohncs left a comment

Choose a reason for hiding this comment

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

The comment in your commit describing what each tsconfig.json is for is quite useful. It'd be nice to also have that information at the top of each file.

I don't think I fully understand the scope of this PR. Are you just trying to get The Lounge to integrate TS into its build process such that TS can be used, or are you also trying to convert as much of the codebase as possible to fully-typed TS?

tsconfig.base.json Outdated Show resolved Hide resolved
@nemchik
Copy link
Contributor

nemchik commented May 7, 2022

The comment in your commit describing what each tsconfig.json is for is quite useful. It'd be nice to also have that information at the top of each file.

https://github.com/microsoft/TypeScript-Babel-Starter/blob/master/tsconfig.json

this file can be used as a template to include all comments (including unused options).

@MaxLeiter
Copy link
Member Author

MaxLeiter commented May 7, 2022

@itsjohncs I was planning to convert the server to typescript and leave the client for incremental changes, but it's looking to be easier to convert everything to typescript to satisfy tsc/eslint/ts-node. There will be a lot left to desire type-wise but it should be a much improved DX experience.

That being said, if you and @bookworm would prefer I split the configuration changes into another PR for ease of review I can see how it goes.

@itsjohncs
Copy link
Member

I suspect we'll want to go the "test it a lot" style of review then 😅. This'll be too big of a change to actually audit I'd think. I think whenever you think it's ready we should just land it onto master and then triage and fix any issues over time afterwards.

@MaxLeiter MaxLeiter removed this from the 4.3.2 milestone Jun 8, 2022
@MaxLeiter MaxLeiter added this to the 5.0 milestone Jun 8, 2022
@MaxLeiter MaxLeiter merged commit dd05ee3 into master Jun 19, 2022
@MaxLeiter MaxLeiter deleted the typescript branch June 19, 2022 00:25
@xPaw xPaw modified the milestones: 5.0, 5.0.0 Jun 21, 2022
@MaxLeiter MaxLeiter modified the milestones: 5.0.0, 4.4.0 Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Meta: Internal This is an internal codebase change (testing, linting, etc.).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants