Skip to content

Conversation

@Smrtnyk
Copy link
Contributor

@Smrtnyk Smrtnyk commented Oct 15, 2025

I have opened an issue microsoft/playwright#37855
because I dislike the solution I came up with, and also I dislike the solution that playwright maintainer suggests.
Lets see if they export the type again, or we have to stick with this.
I would not merge this until the playwright issue is closed

@codesandbox
Copy link

codesandbox bot commented Oct 15, 2025

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@Smrtnyk Smrtnyk closed this Oct 15, 2025
@Smrtnyk Smrtnyk reopened this Oct 15, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Oct 15, 2025

Build Stats

file / KB (diff) bundled minified
fabric 916.833 (0) 301.724 (0)

private async _executeInBrowserImpl(
runInBrowser: any,
context?: any,
): Promise<any> {
Copy link
Member

Choose a reason for hiding this comment

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

yeah i see. This will be hard to remember in a month or two why is in this way. Let me think if some ts notation is a bit better for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i wrote on the issue I opened in playwright that i dislike this approach they suggested
first I had it with just copying the type manually
but I am not sure which one approach is worse

lets see if they reexport the type again, if not then you will have to decide on how to proceed
but in any case, just slapping any type I think is the least optimal solution

@Smrtnyk
Copy link
Contributor Author

Smrtnyk commented Oct 17, 2025

@asturur the playwright issue I filed has been closed without interest to export the type again
so I suggest that we just stick with this, since i don't expect this part of code changing that often
tests are pretty stable and when adding new ones we are not really touching this part of the code

asturur
asturur previously approved these changes Oct 21, 2025
@asturur asturur merged commit 51c5219 into fabricjs:master Oct 21, 2025
14 of 15 checks passed
@Smrtnyk Smrtnyk deleted the chore()-update-playwright branch October 21, 2025 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants