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

Move Head/BodyResourceLinks insertion to compile-time #1585

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

exyi
Copy link
Member

@exyi exyi commented Feb 13, 2023

We used to insert the ResourceLink at the end of
HtmlGenericControl Render, when head or body element was rendered. After this change, the component is inserted compile time into the proper head or body element. This solves the following problems

  • when body contained only literals, LiteralOptimizationVisitor would collapse them into a text: ... binding, which removed all the resources inside
  • when is accidentaly used in the page, resources are rendered on some random place
  • when no body or head tag is found, it would crash Now I'm not sure we want to support this, the error message is reasonable (suggests adding head and body elements) and this "smartness" might now cause problems in markup controls declared as views

The change contains large number of test edits for the following reason

  • new component is inserted into all trees, changing unique ids
  • BodyResourceLink and HeadResourceLinks are added to the pages, so I needed to ignore them when the control tree is being checked.

We used to insert the ResourceLink at the end of
HtmlGenericControl Render, when head or body element
was rendered. After this change, the component is inserted
compile time into the proper head or body element.
This solves the following problems

* when body contained only literals, LiteralOptimizationVisitor
  would collapse them into a `text: ...` binding,
  which removed all the resources inside
* when <body> is accidentaly used in the page, resources are
  rendered on some random place
* when no body or head tag is found, it would crash
  Now I'm not sure we want to support this, the error message
  is reasonable (suggests adding head and body elements)
  and this "smartness" might now cause problems in markup controls
  declared as views

The change contains large number of test edits for the following reason
* new component is inserted into all trees, changing unique ids
* BodyResourceLink and HeadResourceLinks are added to
  the pages, so I needed to ignore them when
  the control tree is being checked.
@tomasherceg tomasherceg added this to the Version 5.0 milestone Sep 8, 2023
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