Skip to content
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

[Visual Testing] Allow object component rendering and unmount previoius component on render #937

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

AlexVelezLl
Copy link
Member

@AlexVelezLl AlexVelezLl commented Feb 11, 2025

Description

This PR:

  • Allow object components in the renderComponent method. This way we can render a custom test component withouth needing to register it in Vue.
  • Clears the previous component that was rendered to clear context of previous test runs.
  • Run visual tests on target branch after PR merge.
    • This is needed since we need the changes to run on the target branch, to have our baseline updated in Percy.

Changelog

  • Description: Allow object component rendering and unmount previoius component on render
  • Products impact: internal
  • Addresses: .
  • Components: .
  • Breaking:
  • Impacts a11y:
  • Guidance:

@bjester
Copy link
Member

bjester commented Feb 19, 2025

@AlexVelezLl Are there any gotchas with this approach? Has anyone else done anything similar?

@AlexVelezLl
Copy link
Member Author

I found this way of serializing functions here: https://stackoverflow.com/questions/7395686/how-can-i-serialize-a-function-in-javascript. I havent seen any gotcha with this approach yet, I tried rendering Custom components with different props and methods and it seems to be working fine. The alternative was to declare every custom component on Vue with Vue.component() in our nuxt plugins, but that would require one step more to use these custom components. What do you think?

@bjester
Copy link
Member

bjester commented Feb 20, 2025

@AlexVelezLl The first potential problem that I see is that this won't account for function scope. Any of the functions could use variables or imports that were defined outside of the function scope, and therefore wouldn't necessarily have access to the same scope if the function was serialized to a string, then deserialized in a different context.

As the code is written currently, I also see these potential pitfalls:

let x = y => y+1;
x.toString() === 'y => y+1' // true

let m = {a: 1, get b() { return this.a }};
structuredClone(m) // returns { a: 1, b: 1 }

let n = {a: 1, b() { return this.a }};
structuredClone(n) // Uncaught: DOMException [DataCloneError]: function() { return this.a } could not be cloned

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