-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
chore(gatsby): Migrate utils/get-public-path.js to ts #22093
Conversation
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 looks great, but since we are changing the export signature there are a handful of files that will need to have their require
statement updated. Do a grok of the codebase for require(get-public-path)
and you'll find a bunch of references that now need to destructure the named export
if (prefixPaths && (assetPrefix || pathPrefix)) { | ||
const normalized = [assetPrefix, pathPrefix] | ||
.filter(part => part && part.length > 0) | ||
.filter((part): part is string => (part ? part.length > 0 : 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.
part is string
is this valid syntax? Why not just part: string
I think this line is wrong. I think you want (part: string): boolean => ..
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.
part is string
is valid syntax called type predicate officially. As far as I know, TS cannot infer types exactly inside Array.prototype.reduce()
so if it is (part: string): boolean => ..
the compiler infers part
in the next map
as string | undefined
example
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.
TIL! V cool, thanks!
Sorry I missed it. I will fix that right away. |
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.
looks great! Thanks!!
Description
Convert
utils/get-public-path
to typescriptDocumentation
Related Issues
Related to #21995