Support meta tags in StaticHTMLRenderer #483

AndrewBarba · 2022-05-19T02:27:12Z

This PR adds the ability to control <head> tags using a new HTMLTitle view and HTMLMeta view. Taking inspiration from Next.js <NextHead>, you can use these views anywhere in your view hierarchy and they will be hoisted to the top <head> section of the html.

Use as a view:

var body: some View {
  VStack {
    ...
    HTMLTitle("Hello, Tokamak")
    HTMLMeta(charset: "utf-8")
    ...
  }
}

Use as a view modifier:

var body: some View {
  VStack {
    ...
  }
  .htmlTitle("Hello, Tokamak")
  .htmlMeta(charset: "utf-8")
}

And the resulting html (no matter where these are used in your view hierarchy):

<html>
  <head>
    <title>Hello, Tokamak</title>
    <meta charset="utf-8">
  </head>
  <body>
    ...
  </body>
</html>

carson-katri · 2022-05-19T12:33:57Z

There was previously some discussion in #269 around designing a way to modify the head through a Scene similar to the Settings scene in SwiftUI:

struct MyApp : App {
  var body: some Scene {
    Head {
      Meta(…)
    }
    WindowGroup(…) {
      ContentView()
    }
  }
}

MaxDesiatov · 2022-05-19T14:02:47Z

@AndrewBarba would do you think about the suggested Head/Meta API that uses types conforming to Scene? I also think that we should support the property attribute for OpenGraph meta tags.

AndrewBarba · 2022-05-19T14:08:58Z

I really like that API, that is certainly my preference. It's very similar to Next.js which I think is a good thing. The static render defines that base html string which I thought was odd but I just tried to fit that. I would much prefer if Scene actually rendered that surrounding html/head/body structure.

MaxDesiatov · 2022-05-19T14:10:39Z

Yeah, that would require bigger changes to the renderer though. Let me know if you'd like to implement them yourself or have any questions. I can pick this up next week otherwise.

AndrewBarba · 2022-05-19T14:15:02Z

I'm happy to do it in this PR I just need some direction (I don't know the codebase too well yet). If we keep the html string, what's the approach to pull out meta tags from the passed in view and render them in the head block?

Also happy to pass this off to you if you already know obvious approach here.

MaxDesiatov · 2022-05-19T14:38:51Z

The hardcoded HTML string is just a hack. I'd suggest replacing the static HTML string with a tree of HTML views that start not from the body, but the top-level <html> tag.

For that you'd initialize rootTarget in public init<V: View>(_ view: V, _ rootEnvironment: EnvironmentValues? = nil) with HTMLTarget(view, htmlTree), where htmlTree is the thing you build from the views, including your new Meta view you'd define with HTML children views in its body. These would be default views you build yourself for that initializer.

You could start with that, and introduce struct Head: Scene in a separate PR.

Let me know if any of this makes sense, I'm happy to provide more guidance if needed.

AndrewBarba · 2022-05-19T15:42:43Z

This kind of makes sense. I would expect a new initializer that takes in a Scene which defines the entire HTML structure - including Meta tags and titles? And then the existing init that just takes in a View would wrap that View with this new base HTML structure. I have some confusion around HTMLTarget, if I make a full htmlTree as the target then all the children seem to be overwritten. Im not sure what the main use of the HTMLTarget struct actually is to be honest.

MaxDesiatov · 2022-05-19T15:47:30Z

I would expect a new initializer that takes in a Scene which defines the entire HTML structure - including Meta tags and titles?

No need for a new initializer, I don't think one would pass Scene values instead of App values, and we already have an initializer for the latter.

I have some confusion around HTMLTarget, if I make a full htmlTree as the target then all the children seem to be overwritten. Im not sure what the main use of the HTMLTarget struct actually is to be honest.

Sorry about the confusion. HTMLTarget is a class that represents tree state the reconciler updates when needed. I misremembered the logic here, you're right, initial HTMLTarget value will always be overwritten. You'll have to modify extension WindowGroup: SceneDeferredToRenderer to wrap view in <head> and <body> tags instead.

AndrewBarba · 2022-05-20T21:53:55Z

@MaxDesiatov @carson-katri Check it out, I refactored PR to use preference keys so now you can drop Title and Meta anywhere in your view hierarchy just like next.js and its hoisted to the top. I'm pretty happy with end result but let me know if you want me to add scripts, styles or anything else. Also very open to feedback on how I organized the files.

var body: some View {
  VStack {
    ...
    Title("Hello, Tokamak")
    Meta(charset: "utf-8")
    ...
  }
}

You can see the result working here: https://tokamak.app.swift.cloud

MaxDesiatov · 2022-05-20T21:56:13Z

Before I look into code just wanted to propose renaming Title and Meta to HTMLTitle and HTMLMeta, which would make it less ambiguous and more consistent with HTMLBody. WDYT?

I know the original issue had the short names, but I wonder if we should prepend HTML in case Title appears in some future SwiftUI version with a different API and semantics.

AndrewBarba · 2022-05-21T00:55:41Z

I agree, I think prefixing makes more sense. I made this change and updated the PR description to show the correct usage. I added usage for both views and view modifiers. SwiftUI on iOS uses a lot of modifiers, ex. .navigationBarTitle(), so I think it's important to support both.

MaxDesiatov · 2022-05-22T20:01:16Z

Sources/TokamakCore/StackReconciler.swift

@@ -58,6 +58,12 @@ public final class StackReconciler<R: Renderer> {
   */
  public let rootTarget: R.TargetType

+  /** A root renderer's main preference store


nit: comment formatting

Suggested change

/** A root renderer's main preference store

/** A root renderer's main preference store.

MaxDesiatov · 2022-05-22T20:02:00Z

Sources/TokamakStaticHTML/Preferences/Preferences.swift

+
+import TokamakCore
+
+internal struct HTMLTitlePreferenceKey: PreferenceKey {


We don't use internal in our codebase as it's the default visibility and is redundant.

Suggested change

internal struct HTMLTitlePreferenceKey: PreferenceKey {

struct HTMLTitlePreferenceKey: PreferenceKey {

MaxDesiatov

There are a few nits to pick, but overall this makes sense. One more question: how does this behave if there are multiple Title views in the same tree? Whichever comes last is applied I guess? Would you be able to add a snapshot test that confirms this? Thanks!

carson-katri

Overall it looks good to me, just needs a couple tests. I think they'd go in TokamakStaticHTMLTests/HTMLTests.swift

carson-katri · 2022-05-22T20:25:07Z

Could you add a test for HTMLMeta too?

AndrewBarba · 2022-05-22T20:25:29Z

Definitely - working on them now

AndrewBarba · 2022-05-22T20:41:18Z

I pushed a test that uses 4 meta tags but only the last one ends up in the resulting html. Currently trying to debug this, I'm pretty sure my implementation is correct so I wonder if there's an issue with the preference store.

AndrewBarba · 2022-05-22T22:21:11Z

Okay the main issue here is that every mount target gets its own preference store and then we are trying to merge upwards anytime a child store changes. This is problematic because the merge only saves the value of the child and doesn't maintain any values in the parent for the same key.

I was able to fix this by:

Change _PreferenceStore to a class
Initialize MountedElement.preferenceStore to the parents store or create a new one
Remove _PreferenceStore.merge as it should not be needed

I can push this up if you think it makes sense, otherwise I'm open to other approaches.

public class MountedTarget {
  var preferenceStore: _PreferenceStore

  ...

  init(_ app: _AnyApp, _ environmentValues: EnvironmentValues, _ parent: MountedElement<R>?) {
    element = .app(app)
    self.parent = parent
    self.preferenceStore = parent?.preferenceStore ?? .init()
    self.environmentValues = environmentValues
    viewTraits = .init()
    updateEnvironment()
  }
}

MaxDesiatov · 2022-05-23T07:38:51Z

Thanks for the investigation! IIRC @carson-katri is the author of preference store code, so I'll defer to him on this matter if he has a moment to review this.

carson-katri · 2022-05-23T13:47:29Z

So does this make the child take all the values of the parent's preference store? If so, I don't think this behavior is correct as preference values flow up the tree not down. Similar to how an environment value from a child is not set on the parent, a preference value from a parent is not set on the child.

AndrewBarba · 2022-05-23T13:57:16Z

Ah yes you are right. Don't think we have tests for this so I will add them.

So I think the approach to solve that is add a weak parent store to the _PreferenceStore. Each mount target will go back to getting its own store but the pref store will be updated to modify its parent store anytime one of its keys changes. This is still a better solution to merge, because the issue with merge is it doesn't know which keys changed, and so even with some fancy generic work to deep merge keys upwards, it was over adding values to the parent store because it was doing a full merge on ever didSet. If we control the upstream changes to only happen on inserting a key this should be more reliable.

…ee preference keys they should not have access to

AndrewBarba · 2022-05-23T14:52:45Z

@carson-katri I pushed that up, take a look let me know what you think. Im working on a test to cover that case now

AndrewBarba · 2022-05-23T17:37:55Z

Okay just pushed a test that I think correctly covers what you described.

….MetaTag public with better initializer.

carson-katri

That looks good to me, thanks!

MaxDesiatov

Overall this is great, one small change request: could you run SwiftFormat, and potentially have a pre-commit hook installed as described in the "Coding Style" section of CONTRIBUTING.md? Some of the newly added files use 4 spaces for indentation, while we use 2 spaces everywhere in the project.

Thanks!

AndrewBarba · 2022-05-23T19:53:02Z

Yes definitely. Xcode is so annoying with the indents, I thought I caught them all. Will get this fixed

MaxDesiatov · 2022-05-23T19:54:17Z

Don't worry about catching them in Xcode manually. When I edit in Xcode I just write some messy inconsistent indentation, and it's all reformatted with pre-commit on every commit automatically 🙂

AndrewBarba · 2022-05-23T19:57:42Z

Yeah wow - this is so much better. My bad for not reading that code style doc earlier

MaxDesiatov

Lovely, thanks!

AndrewBarba added 3 commits May 18, 2022 22:25

Support meta tags in StaticHTMLRenderer

dc09aa8

Fix meta line spacing

2845c9e

Add back static title var

fc11418

MaxDesiatov added the enhancement New feature or request label May 19, 2022

Add HTMLMetaTag.property

8de0f62

AndrewBarba added 2 commits May 20, 2022 17:46

Refactor to use preference keys

20cde70

Copyright

2d0f478

HTML title and meta modifiers

4b904c9

MaxDesiatov requested a review from carson-katri May 21, 2022 10:04

MaxDesiatov reviewed May 22, 2022

View reviewed changes

AndrewBarba added 2 commits May 22, 2022 16:15

Nits

8a68ea5

Double title snapshot test

d40fe29

carson-katri requested changes May 22, 2022

View reviewed changes

AndrewBarba added 2 commits May 22, 2022 16:33

HTMLMeta tests

25c5b89

Test meta all - currently failing

edd2ea9

Move _HTMLMeta enum to HTMLView file

7d20972

AndrewBarba added 3 commits May 22, 2022 18:28

Convert use a single preference store per view hierarchy

5054fd3

Remove redundant internal

3df1cf0

Lint

338725b

Add _PreferenceStore.parent to avoid issue where child stores would s…

fe93e4b

…ee preference keys they should not have access to

MaxDesiatov requested a review from carson-katri May 23, 2022 15:26

Preference propagation test

d4972fc

Make preference keys public so any view can read value. Make HTMLMeta…

dd8ad03

….MetaTag public with better initializer.

carson-katri previously approved these changes May 23, 2022

View reviewed changes

MaxDesiatov requested changes May 23, 2022

View reviewed changes

Formatting

a08d7e4

AndrewBarba dismissed carson-katri’s stale review via a08d7e4 May 23, 2022 19:56

MaxDesiatov approved these changes May 23, 2022

View reviewed changes

MaxDesiatov enabled auto-merge (squash) May 23, 2022 20:00

MaxDesiatov merged commit 2ba5488 into TokamakUI:main May 23, 2022

ezraberch mentioned this pull request May 24, 2022

Revert "Add html5 doctype to static renderer (#486)" #487

Merged

carson-katri mentioned this pull request Jul 6, 2022

Support <meta>/<title> in Fiber renderers #503

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support meta tags in StaticHTMLRenderer #483

Support meta tags in StaticHTMLRenderer #483

AndrewBarba commented May 19, 2022 •

edited

Loading

carson-katri commented May 19, 2022

MaxDesiatov commented May 19, 2022

AndrewBarba commented May 19, 2022 •

edited

Loading

MaxDesiatov commented May 19, 2022

AndrewBarba commented May 19, 2022 •

edited

Loading

MaxDesiatov commented May 19, 2022 •

edited

Loading

AndrewBarba commented May 19, 2022 •

edited

Loading

MaxDesiatov commented May 19, 2022 •

edited

Loading

AndrewBarba commented May 20, 2022

MaxDesiatov commented May 20, 2022 •

edited

Loading

AndrewBarba commented May 21, 2022 •

edited

Loading

MaxDesiatov May 22, 2022

MaxDesiatov May 22, 2022

MaxDesiatov left a comment

carson-katri left a comment

carson-katri commented May 22, 2022

AndrewBarba commented May 22, 2022

AndrewBarba commented May 22, 2022

AndrewBarba commented May 22, 2022 •

edited

Loading

MaxDesiatov commented May 23, 2022 •

edited

Loading

carson-katri commented May 23, 2022

AndrewBarba commented May 23, 2022

AndrewBarba commented May 23, 2022 •

edited

Loading

AndrewBarba commented May 23, 2022

carson-katri left a comment

MaxDesiatov left a comment

AndrewBarba commented May 23, 2022

MaxDesiatov commented May 23, 2022

AndrewBarba commented May 23, 2022

MaxDesiatov left a comment

	/** A root renderer's main preference store
	/** A root renderer's main preference store.


		import TokamakCore

		internal struct HTMLTitlePreferenceKey: PreferenceKey {

	internal struct HTMLTitlePreferenceKey: PreferenceKey {
	struct HTMLTitlePreferenceKey: PreferenceKey {

Support meta tags in StaticHTMLRenderer #483

Support meta tags in StaticHTMLRenderer #483

Conversation

AndrewBarba commented May 19, 2022 • edited Loading

carson-katri commented May 19, 2022

MaxDesiatov commented May 19, 2022

AndrewBarba commented May 19, 2022 • edited Loading

MaxDesiatov commented May 19, 2022

AndrewBarba commented May 19, 2022 • edited Loading

MaxDesiatov commented May 19, 2022 • edited Loading

AndrewBarba commented May 19, 2022 • edited Loading

MaxDesiatov commented May 19, 2022 • edited Loading

AndrewBarba commented May 20, 2022

MaxDesiatov commented May 20, 2022 • edited Loading

AndrewBarba commented May 21, 2022 • edited Loading

MaxDesiatov May 22, 2022

Choose a reason for hiding this comment

MaxDesiatov May 22, 2022

Choose a reason for hiding this comment

MaxDesiatov left a comment

Choose a reason for hiding this comment

carson-katri left a comment

Choose a reason for hiding this comment

carson-katri commented May 22, 2022

AndrewBarba commented May 22, 2022

AndrewBarba commented May 22, 2022

AndrewBarba commented May 22, 2022 • edited Loading

MaxDesiatov commented May 23, 2022 • edited Loading

carson-katri commented May 23, 2022

AndrewBarba commented May 23, 2022

AndrewBarba commented May 23, 2022 • edited Loading

AndrewBarba commented May 23, 2022

carson-katri left a comment

Choose a reason for hiding this comment

MaxDesiatov left a comment

Choose a reason for hiding this comment

AndrewBarba commented May 23, 2022

MaxDesiatov commented May 23, 2022

AndrewBarba commented May 23, 2022

MaxDesiatov left a comment

Choose a reason for hiding this comment

AndrewBarba commented May 19, 2022 •

edited

Loading

AndrewBarba commented May 19, 2022 •

edited

Loading

AndrewBarba commented May 19, 2022 •

edited

Loading

MaxDesiatov commented May 19, 2022 •

edited

Loading

AndrewBarba commented May 19, 2022 •

edited

Loading

MaxDesiatov commented May 19, 2022 •

edited

Loading

MaxDesiatov commented May 20, 2022 •

edited

Loading

AndrewBarba commented May 21, 2022 •

edited

Loading

AndrewBarba commented May 22, 2022 •

edited

Loading

MaxDesiatov commented May 23, 2022 •

edited

Loading

AndrewBarba commented May 23, 2022 •

edited

Loading