-
Notifications
You must be signed in to change notification settings - Fork 2
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
Share types across client and server #190
Share types across client and server #190
Conversation
* definitely not working but want to work on desktop
* get server dev tools working with common package * still need to get production build working
* except resources aren't copied over to dist folder
…typescript enums have some downsides and unions are preferred
…des them in the bundle
* still need to migrate options, midgameReconnect * add typechecking to socketioClient
* refactor options a bit to make both sides agree * fix client build (resolve common imports)
* fix typing errors that ensued from that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't see any issues, though I feel like the change of removing objects (i.e. stage.lobby => 'lobby') and replacing it with strings may make potential future features harder to implement
packs: Record<string, boolean>; | ||
customPrompts: string; | ||
}; | ||
import { SettableOptions } from ':common/options'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this and all imports from :common, do you need to declare type? i.e import { type SettableOptions }
? Is there any reason why this has to be different from how you are importing :common/player
or :common/match
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the client side I had a linter setting or typescript setting that expected explicit type imports. Not sure how it slipped through, maybe that setting has been turned off. This was the case for all types not just common ones
active: boolean; | ||
}; | ||
|
||
export type Stage = 'lobby' | 'response' | 'selection' | 'matching' | 'endRound'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kajgrant note the Stage type isn't a string, it's a union of string values. There's some weird reasons that string unions tend to be preferred over const enums in typescript.
No description provided.