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.

Sign up for GitHub

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

Jump to bottom

bug: E2EPage setContent removes all script and style tags from the page #3229

Open
3 tasks done
Trendy opened this issue Feb 1, 2022 · 3 comments · May be fixed by #3257
Open
3 tasks done

bug: E2EPage setContent removes all script and style tags from the page #3229

Trendy opened this issue Feb 1, 2022 · 3 comments · May be fixed by #3257
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil Has Workaround This PR or Issue has a work around detailed within it.

Comments

@Trendy
Copy link

Trendy commented Feb 1, 2022

Prerequisites

Stencil Version

2.13

Current Behavior

Sometimes there may be external dependencies that many tests will need, so adding a script or stylesheet to the E2EPage could be defined in a beforeEach/beforeAll. However, if you are using E2EPage.setContent in an individual test, it will remove all previously added script and styles, leading to some confusing behavior.

Expected Behavior

Calling E2EPage.setContent will not remove previous scripts and styles.

Steps to Reproduce

  1. Create an end-to-end test for a component.
  2. In the test, add a beforeEach that creates the page, and calls page.addScriptTag to add a dependency (i.e. bootstrap, shoelace, etc).
  3. In the test, add a describe and it, call page.setContent.
  4. Launch the test in browser mode with debugging enabled, inspect the <head>.

Code Reproduction URL

https://github.com/Trendy/stencil-set-content-bug

Additional Information

No response
@ionitron-bot ionitron-bot bot added the triage label Feb 1, 2022
@rwaskiewicz rwaskiewicz added Bug: Validated This PR or Issue is verified to be a bug within Stencil Feature: Testing and removed triage labels Feb 2, 2022
@rwaskiewicz
Copy link
Member

👋

I've confirmed that this is a bug and have labeled it so that it gets ingested into our internal issue tracker & backlog. Thanks for the great reproduction case, it's very much appreciated!!

@Trendy
Copy link
Author

Trendy commented Feb 2, 2022

No problem!

I took a stab at fixing it last night as well, but I couldn't get the build to work linked into my project locally. I'll reach out to the Slack for some help with that later on today.

If anyone else is having issues with this, I was able to get around it by adding the script tags after the setContent call. The dependency was another collection of UI components though, so your milage may vary if your component depends on external libraries for rendering or connectedCallbacks.

@rwaskiewicz rwaskiewicz added the Has Workaround This PR or Issue has a work around detailed within it. label Feb 2, 2022
Trendy added a commit to Trendy/stencil that referenced this issue Feb 25, 2022
@Trendy
fix(puppeteer-page): add existing head content to page
323dee8 
Read the existing head element content, add that content to the e2e page that is being created. This prevents a user losing script or style tags that may have been added to the page before using the setContent method. Fixes ionic-team#3229
Trendy added a commit to Trendy/stencil that referenced this issue Feb 25, 2022
@Trendy
fix(puppeteer-page): add existing head content to page
1b00732 
Read the existing head element content, add that content to the e2e page that is being created. This prevents a user losing script or style tags that may have been added to the page before using the setContent method. Fixes ionic-team#3229
@Trendy Trendy linked a pull request Feb 25, 2022 that will close this issue
Open
14 tasks
@Trendy
Copy link
Author

Trendy commented Feb 27, 2022

Update:I revisited this issue, and came up with a solution. The pull request is failing automated tests, currently. I'll attempt to fix those in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil Has Workaround This PR or Issue has a work around detailed within it.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(puppeteer-page): add existing head content to page Trendy/stencil
2 participants
@rwaskiewicz @Trendy