-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
pr05 Typescript Migration #10: Setup Server TS Dependencies & migrate instance of server file #3636
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
base: develop
Are you sure you want to change the base?
pr05 Typescript Migration #10: Setup Server TS Dependencies & migrate instance of server file #3636
Conversation
…ver folder ts extensions after client folder
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.
great work!!!
@@ -5,12 +5,10 @@ | |||
* @param {String} string | |||
* @param {String} replacer (optional) character to replace invalid characters | |||
*/ | |||
function generateFileSystemSafeName(string, replacer) { | |||
export function generateFileSystemSafeName(string: string, replacer: string) { |
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.
is there any way we can rename this from string to something else? i assume it's not causing a bug but it just makes me nervous haha
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.
Maybe "unsafeName" or more general, like "name"?
window.process.env.UPLOAD_LIMIT = ${process.env.UPLOAD_LIMIT ? `${process.env.UPLOAD_LIMIT}` : undefined}; | ||
window.process.env.TRANSLATIONS_ENABLED = ${process.env.TRANSLATIONS_ENABLED === 'true' ? true : false}; | ||
window.process.env.LOGIN_ENABLED = ${ | ||
process.env.LOGIN_ENABLED !== 'false' |
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.
no big deal either way but would you want to use the helper utils you wrote for this case & the ones below?
export function parseBoolean( |
the only thing is you'd have to move the utility file into common/
"target": "ES2022", | ||
"module": "commonjs", | ||
"lib": ["ES2022"], | ||
"types": ["node", "jest"], |
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.
This doesn't need to be changed at all, but just wanted to add a super small style note that the repo’s convention is to skip trailing commas!
pr05 Typescript Migration 10: Set up TS dependencies to the server folder & migrate an instance of the server folder.
Changes:
typecheck:server
to package.json commandsOther: