Influence HTML head from Blazor #23833

MackinnonBuck · 2020-07-10T01:21:08Z

Summary of the changes

Added Title component to modify the document title.
Added Meta component to add or modify meta tags.
Added Link component.
Wrote functional and E2E tests.
Added prerendering support

Implementation details

I've changed my implementation to be simpler at the cost of some functionality. If users request additional functionality, we should consider taking pieces from this.

Usage sample:

<Title Value="@title" />
<Meta name="description" content="Modifying the head from a Blazor component." />
<Link href="main.css" rel="stylesheet" />

Note that these components don't require a surrounding Head component.

Addresses #10450

src/Components/Web.Extensions/src/HeadManagement/Link.cs

src/Components/Web.Extensions/src/wwwroot/headManager.js

SteveSandersonMS · 2020-07-15T14:32:55Z

This is a great start. Thanks @MackinnonBuck! A couple of thoughts:

Do we definitely want <Link>? Because we're working towards a CSS isolation feature in .NET 5 which will give people a more first-class way to include custom CSS within a component rather than needing any special tags. And for people whose goal is to dynamically reference an external .css file, isn't it equally good just to use a regular <link> element in a component? I know that <Link> will add the element to <head> whereas a regular <link> would put it in the middle of the document, but since this is being added dynamically I'm not sure it makes any difference to the end result. So maybe we should not have <Link>.
I don't think the hierarchical behavior will be quite right in all cases (update actually I am siding with your design now!):
- The approach here relies on parents always being rendered before children. While it's true that parents are initially rendered before their descendants, it's entirely possible for a parent to be re-rendered later without necessarily re-rendering its descendants. Since you overwrite the title on each call to OnParametersSet, the parent could overwrite the title written by one of its descendants. Additionally, when a <Title> is removed, it doesn't set the title back to what it was previously, so you can't just temporarily set a title using an @if block.
- Update: The more I think about this, the less I'm convinced there's any solution that is simultaneously "robust" while also being nice to use. To make it really robust you'd need to make people wrap <Title> around the contents of their page so they were really nested inside each other, not on sibling branches, then there's a well-defined order. But the usability would suck. So maybe the design you have is already the best choice, and we should just document something like "Each <Title> overwrites the previous document title when rendered, so be sure to only render one at a time." and discourage people from having multiple <Title> components at different levels of their hierarchy.

MackinnonBuck · 2020-07-15T21:55:02Z

Thanks for the feedback, @SteveSandersonMS! My main reason for adding <Link> was to match functionality provided by similar libraries for React. For example, react-helmet supports base, bodyAttributes, htmlAttributes, link, meta, noscript, script, style, and title. It should be trivial to add our own support for more tags so I think it's fine to remove <Link> unless others think there's a good reason to leave it in.

jimspillane · 2020-07-15T22:35:26Z

The link element can be used for a number of other link types such as rel="icon" that are not body-ok types.

pranavkm

We discussed removing special code paths for prerendering and yoloing it all the time. This should remove the dependency on Server.Circuits from this project.

src/Components/Web.Extensions/src/HeadManagement/Link.cs

src/Components/Web.Extensions/src/HeadManagement/TitleElement.cs

src/Components/Web.Extensions/src/wwwroot/headManager.js

pranavkm · 2020-07-15T22:48:21Z

src/Components/Web.Extensions/src/HeadManagement/HeadTagBase.cs

+        protected override void OnInitialized()
+        {
+            _headManager = ServiceProvider.GetHeadManager() ??
+                throw new InvalidOperationException($"{GetType()} requires a {typeof(HeadManager)} service.");


@SteveSandersonMS \ @javiercn, would we consider having a way for Inject to tell you what to do to fix this?

[Inject(Error = "Did you forget to invoke AddBlah()?")]

It shouldn't be too much work to include that in the diagnostics here:

aspnetcore/src/Components/Components/src/ComponentFactory.cs

Line 80 in 0ec15ea

throw new InvalidOperationException($"Cannot provide a value for property " +

SteveSandersonMS · 2020-07-15T22:50:40Z

src/Components/Web.JS/src/Rendering/LogicalElements.ts

+    if (grandchildrenArray) {
+      while (grandchildrenArray.length > 0) {
+        removeLogicalChild(childToRemove, 0);
+      }


This is interesting. Was there a bug before? What scenario triggered it?

I encountered a bug where grandchildrenArray was undefined if a component rendered a comment during prerendering.

src/Components/Web.Extensions/src/HeadManagement/HeadManagementJSRuntimeExtensions.cs

src/Components/Web.Extensions/src/HeadManagement/HeadManagementRenderTreeBuilderExtensions.cs

pranavkm

This looks great. Could you rebase this on to the release/5.0-preview8 branch?

src/Components/Web.Extensions/src/HeadManagement/HeadTagBase.cs

src/Components/Web.Extensions/src/WebExtensionsServiceCollectionExtensions.cs

src/Components/Web.Extensions/src/HeadManagement/HeadManagementRenderTreeBuilderExtensions.cs

src/Components/Web.Extensions/src/Microsoft.AspNetCore.Components.Web.Extensions.csproj

src/Components/test/testassets/BasicTestApp/ModifyHeadComponent.razor

javiercn · 2020-07-22T09:52:07Z

src/Components/test/testassets/BasicTestApp/PrerenderedHeadComponent.razor

+<p>
+    Meta:<br />
+    <input id="input-meta-binding" @bind="metaContent" placeholder="Set the meta content" />
+    <Meta id="meta-with-bindings" content="@metaContent" />


What's the IDE experience with this? We've had problems in the past with Components that overlapped with HTML tag names, since Meta is a valid HTML5 tag (they are case-insensitive)

It's a good point. Based on experiences with <Button>/<button>, it seems likely that VS may interfere with typing <Title> and try to auto-correct it to <title>.

While this is annoying, I don't think we should block the PR on it, because there isn't a great alternative right now, and developers can self-correct this, and eventually the IDE can do a better job with it.

Also a better solution still would be if we implemented #22194. Then it would be possible for people not to have a @using directive for the full namespace here, but instead type things like:

<Head.Title>My title</Head.Title>

However that would only work if the namespaces containing these elements was something like Microsoft.AspNetCore.Components.Web.Extensions.Head.

Do you think it might be worth putting them in that namespace proactively so that we can get this developer experience eventually when #22194 is implemented? In the short term developers will still get the same experience of typing <Title>Hey</Title> as long as they have a @using for the full namespace. This also fixes one other small concern I had that these component names are super-generic and people might have their own reasons for creating unrelated components called <Title>/<Link> etc.

Interested in opinions from @javiercn as well as @MackinnonBuck here!

I agree with @SteveSandersonMS's suggestion to proactively put the components in a Head namespace. It would make good use of #22194 (when implemented) without making the current user experience worse.

src/Components/test/E2ETest/ServerExecutionTests/PrerenderingTest.cs

javiercn

I have some minor comments, but overall looks great!

SteveSandersonMS

Fantastic work, @MackinnonBuck! This looks great to me.

As with @javiercn there are a couple of remaining small comments but hopefully this should be able to merge really soon.

…etcore into t-mabuc/influence-head

amis92 · 2020-07-23T21:45:58Z

@MackinnonBuck I'm sorry to bother you, but I've browsed this PR and haven't found an answer: how is the prerendering actually working here? From what I see, there are only comments added to the render tree, and all actual DOM work is done in JavaScript. Is JS being run now as part of prerender?

I ask because in the #10450 there was some talk about using such a feature for impacting social previews. Would such scenario be covered by this PR?

javiercn · 2020-07-23T21:52:46Z

@amis92 the way it works is during prerendering the content gets emitted as a comment into the DOM and then a small piece of JS updates it when the page loads on the browser.

amis92 · 2020-07-23T22:00:38Z

Ah. So no game, then. It's not actually prerendered for static HTML processing like link previews on websites.

sfmskywalker · 2020-08-21T13:41:56Z

Fantastic, this PR adds just the thing I was looking for :D

Seeing that this PR was merged into https://github.com/dotnet/aspnetcore/tree/release/5.0-preview8, I assumed I would be able to find it on the aspnetcore preview feed in the microsoft.aspnetcore.components.web\5.0.0-preview.8.20364.16 package, but I can't seem to find anything containing these bits. No doubt I'm looking in the wrong place or misunderstanding what branches get build where.

Any pointers on where I might try these things out (ideally using a package feed)?

sfmskywalker · 2020-08-21T13:46:55Z

Typical. I did one more attempt at finding it after writing the above, and of course I found it. I was looking on the Installed tab in Visual Studio rather than on the Browse tab 😅 One day I'll get the hang of this VS NuGet UI 🤞

ghost added the area-blazor Includes: Blazor, Razor Components label Jul 10, 2020

MackinnonBuck requested review from pranavkm, javiercn and SteveSandersonMS July 10, 2020 01:21