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

fix(runtime): fix NotFoundError in addStyle function with referenceNode parent check #6107

Merged

Conversation

hbeyan
Copy link
Contributor

@hbeyan hbeyan commented Jan 20, 2025

What is the current behavior?

addStyle function causes NotFoundError in for some components in our micro front end app.

GitHub Issue Number: 6106

What is the new behavior?

referenceNode's parentNode is checked to prevent insertBefore throwing NotFoundError method.
If the referenceNode is not a direct child of the element receiving the styleNode as the child, referenceNode is not used.

Documentation

Does this introduce a breaking change?

  • Yes
  • No

Testing

Other information

@hbeyan hbeyan requested a review from a team as a code owner January 20, 2025 18:23
@christian-bromann
Copy link
Member

@hbeyan thanks for raising the issue. Mind adding a WebdriverIO test for this to ensure we don't regress on this?

@hbeyan
Copy link
Contributor Author

hbeyan commented Jan 23, 2025

@hbeyan thanks for raising the issue. Mind adding a WebdriverIO test for this to ensure we don't regress on this?

Unfortunately I can only reproduce the issue in micro front-end app, I wasn't able to reproduce it with WebdriverIO tests

@christian-bromann
Copy link
Member

, I wasn't able to reproduce it with WebdriverIO tests

Can you reproduce it on a Stencil Starter app? npm init stencil component exampleProject

@johnjenkins
Copy link
Contributor

I guess it's quite a logical check to do either way (that the parent node include the reference node)?

I don't really understand how it can happen ... it's looking for <link /> or <style /> elements in the head ... and nothing in the head can be nested... so it doesn't really make sense, but I guess there's no harm having an extra check

@hbeyan
Copy link
Contributor Author

hbeyan commented Jan 24, 2025

, I wasn't able to reproduce it with WebdriverIO tests

Can you reproduce it on a Stencil Starter app? npm init stencil component exampleProject

reproduction project is linked in the issue #6106

Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

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

👍

@christian-bromann christian-bromann added this pull request to the merge queue Jan 25, 2025
@christian-bromann christian-bromann removed this pull request from the merge queue due to a manual request Jan 25, 2025
@christian-bromann christian-bromann merged commit 26ceed6 into ionic-team:main Jan 25, 2025
71 checks passed
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.

3 participants