-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
feat(): Missing features from fabric 5 and types exports #8954
Conversation
Build Stats
|
…ric.js into types-cleanup-while-working
…ric.js into types-cleanup-while-working
@ShaMan123 i found some type issues in the build after the introduction of textStyleObject type, so i tried to fix it here. |
) { | ||
console.error(chalk.redBright(warning.message)); | ||
throw Object.assign(new Error(), warning); | ||
} |
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.
@ShaMan123 let's have the build error on default if types are broken.
We can comment this locally when we need to work in a broken state with watchers, but i feel better if the pipeline is strict about this, otherwise we found ourselves with unexpected cleanups that we can't see when we code review the PRs
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.
I agree
That sourcemap warning must be resoled as well
@ShaMan123 i have to push this in a beta7 now for work reason i can't defer for my own peace of mind. I expect that we need to get back to some points from here. |
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 is fine
) { | ||
console.error(chalk.redBright(warning.message)); | ||
throw Object.assign(new Error(), warning); | ||
} |
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.
I agree
That sourcemap warning must be resoled as well
const { styles, fontFamily } = obj; | ||
if (fontList[fontFamily] || !fontPaths[fontFamily]) { | ||
return; | ||
} | ||
fontList[fontFamily] = true; | ||
if (!obj.styles) { | ||
if (!styles) { | ||
return; | ||
} | ||
Object.values(obj.styles).forEach((styleRow) => { | ||
Object.values(styleRow).forEach((textCharStyle) => { | ||
fontFamily = textCharStyle.fontFamily; | ||
Object.values(styles).forEach((styleRow) => { | ||
Object.values(styleRow).forEach(({ fontFamily = '' }) => { |
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.
I have touched this in my cleanup #8952
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.
i think the only thing i really changed is es6 leverage
@@ -164,6 +145,7 @@ export abstract class StyledText< | |||
graphemeCount += this._textLines[i].length; | |||
} | |||
if (allStyleObjectPropertiesMatch && stylesCount === graphemeCount) { | |||
// @ts-expect-error conspiracy theory of TS |
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.
And don't know if to laugh or cry
These annoying errors
property | ||
); | ||
const styleObject = obj[p1][p2] || {}, | ||
stylePropertyHasBeenSet = styleObject[property] !== undefined; |
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.
#8953 => but the rest are typeof meanwhile.
Motivation
Converting a small app from fabric 5 to 6 i noticed the following issues:
Also added some typings