Allow Populating Head Content #66

ronco · 2015-09-23T04:54:28Z

This strips down the great work done by @jpadilla in #49 to just serialize the (newly exposed) simple dom head element. This allows apps and addons to populate the head element as they see fit while running under fastboot.

I'll also open up a PR against ember-fastboot-server with the necessary changes for replacing the head content.

This assumes we're able to merge ember-fastboot/simple-dom#12

- just serialize the head tag from the document directly - leave population of the tag to application code

ronco · 2015-09-23T04:58:16Z

Here is the ember-fastboot-server PR companion to this PR.

2 of the failing tests on this PR are fixed when using the version of ember-fastboot-server provided with the other PR. The other two failures appear to be present on master.

Please let me know how I can help finish this off. I know there are a lot of moving parts now that the project has split.

rwjblue · 2015-09-23T13:50:13Z

Looks good to me.

jpadilla · 2015-09-23T14:02:52Z

@ronco thanks for taking over this and making it happen.

ronco · 2015-09-23T14:56:29Z

@jpadilla No problem. I'm giving a talk at our local meetup next week on searchable ember apps (ie meta tags and pre-rendering). Seemed like a good opportunity to demo fastboot.

ronco · 2015-09-25T02:40:25Z

@rwjblue
Note: I just noticed this is rendering nested body and head tags. For example with the first simple acceptance test you get the following HTML:

<!DOCTYPE html>
<html>
  <head>
    <meta charset="utf-8">
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
    <title>Dummy</title>
    <meta name="viewport" content="width=device-width, initial-scale=1">

    <base href="/" />
<head><title>Application Route -- Title</title><meta name="description" content="something here"><meta property="og:image" content="http://placehold.it/500x5
00"></head>

    <link rel="stylesheet" href="assets/vendor.css">
    <link rel="stylesheet" href="assets/dummy.css">


  </head>
  <body>
    <body><div id="ember332" class="ember-view"><h2 id="title">Welcome to Ember.js</h2>

<!---->
</div></body>

    <script src="assets/vendor.js"></script>
    <script src="assets/dummy.js"></script>


  </body>
</html>

Note the <head> tag within the outer <head> tag, and likewise the same for body. The head tag issue is introduced with this PR, but the <body> issue exists on master.

I think the best way to address it would be for SimpleDOM.HTMLSerializer.serialize to have an option to only serialize children nodes (ie not include self). Thoughts?

rwjblue · 2015-09-30T22:04:46Z

Seems like a reasonable solution off the cuff, mind digging in and seeing if there is already a way to do this in SimpleDOM (and submit a PR if not)?

ronco · 2015-09-30T22:17:49Z

Will do.

Probably won't get to it till tomorrow night. I'm giving a meetup talk on
prerendering and fastboot tonight.
On Wed, Sep 30, 2015 at 4:04 PM Robert Jackson notifications@github.com
wrote:

Seems like a reasonable solution off the cuff, mind digging in and seeing
if there is already a way to do this in SimpleDOM (and submit a PR if not)?

—
Reply to this email directly or view it on GitHub
#66 (comment)
.

rwjblue · 2015-10-02T13:35:50Z

@ronco has submitted a PR to SimpleDOM (ember-fastboot/simple-dom#18) to fix that issue.

@ronco - Can you update this PR to point at your fork/branch of SimpleDOM so we can play around with things here a bit easier?

ronco · 2015-10-02T16:49:35Z

@rwjblue updated this PR to point to my simple-dom fork and my fastboot-server fork.

Note fastboot is throwing an error on the latest ember canary:

ReferenceError: document is not defined
    at new BootOptions (<anonymous>:14405:25)
    at _emberRuntimeSystemObject.default.extend._bootSync (<anonymous>:14105:19)
    at <anonymous>:14082:30
    at Object.initializePromise (<anonymous>:63015:7)
    at new Promise (<anonymous>:64865:21)
    at _emberRuntimeSystemObject.default.extend.boot (<anonymous>:14081:27)
    at <anonymous>:15504:40
    at tryCatch (<anonymous>:62965:14)
    at Object.invokeCallback (<anonymous>:62980:15)
    at <anonymous>:65106:25

I think it's this line causing the issue in the node environment. I was able to test out this whole stack with an older canary checkout (2.2.0-canary+1a2cd166).

rwjblue · 2015-10-02T18:30:45Z

@chancancode - I believe that this change was introduced in emberjs/ember.js#12394, and given that you are familiar with fastboot (and that PR was submitted to make the visit api much better) I am hoping that you might have a few pointers on how we need to update fastboot....

chancancode · 2015-10-02T18:38:11Z

eh, that's one of my TODOs. I'll send a PR to update fastboot.

ronco · 2015-12-09T04:08:37Z

@rwjblue @tomdale I'm about to start rebasing this for the new visit api updates (and make any necessary implementation updates).

However on master currently tests are failing in the under-test smoke apps when run against canary. Updating the acceptance helper to run beta instead seems to fix.

rwjblue · 2015-12-09T13:53:58Z

@ronco - There was a very recent change/update to canary (which will be pulled into beta) that needs to be incorporated here. I'm sure @tomdale would have insight..

tomdale · 2015-12-09T14:27:37Z

@rwjblue @ronco I believe the only change is that the protocolForURL and setMorphHTML monkeypatches are no longer required by consuming implementations. Everything else should be the same, but do let me know what problems you run into.

ronco · 2015-12-09T15:42:40Z

@rwjblue @tomdale Thanks for the info. I'll see if I can get that fixed before tackling this rebase. Looking at the work @chancancode did for the visit api here & on the server I think this feature will be much easier to implement than the original attempt.

tomdale · 2015-12-09T17:18:35Z

My thinking about how to implement this:

Provide a head-metadata service
Routes are injected with the head-metadata service and can set properties on it based on the properties of the models they load.
Users can implement a head.hbs template in their project.
During FastBoot, we boot the application as normal. Once the routing promise chain has completed, the head-metadata service will be populated with any relevant properties.
Before rendering the application's templates, we load head.hbs and render it in a stand-alone component whose model property is set to the head-metadata service. The serialized HTML for that component is inserted into the returned HTML. (We do it before rendering so we do not block streaming responses in the future, as <head> content must be sent first.)

Example

I want to support Open Graph in my application that shows rendered articles.

// app/routes/article.js
export default Route.extend({
  model(params) {
    return fetch(`http://api.example.com/articles/${params.article_id}.json`)
      .then(response => response.json());
  },

  afterModel(article) {
    let headMetadata = this.get('headMetadata');
    headMetadata.set('title', article.title);
    headMetadata.set('image', {
      url: article.thumbnailURL,
      width: article.thumbnailWidth,
      height: article.thumbnailHeight
    });
  }
});

{{!-- app/templates/head.hbs --}}
<title>{{model.title}}</title>
<meta property="og:site_name" content="Hubbub">
<meta property="og:title" content={{model.title}}>
<meta property="og:image" content={{model.image.url}}>
<meta property="og:image:width" content={{model.image.width}}>
<meta property="og:image:height" content={{model.image.height}}>

ronco · 2015-12-09T19:35:27Z

That's a real interesting approach. I really like the idea of giving users access to that template for expressively building their head content.

Would you be able to make full use of components and helpers in that template?

In my addon I've created a component with a template to accomplish this which is appended to the head. It then has sub components for rendering the individual head tags. I think it makes a good example for this implementation.

Do we need any changes to ember or ember-cli-fastboot for this? Would an addon with this service accomplish this combined with my fastboot-server PR to insert the whole head rather than just title?

ronco · 2015-12-10T06:08:28Z

Hey @tomdale,

I'm getting this error when trying to run the fastboot test suite against ember canary:

Error while processing route: index DOM Helper could not find valid URL parsing mechanism Error: DOM Helper could not find valid URL parsing mechanism
    at installEnvironmentSpecificMethods (<anonymous>:14270:13)
    at new DOMHelper (<anonymous>:13851:5)
    at DOMHelper.EmberDOMHelper (<anonymous>:22948:24)
    at Object.registry.register.create (<anonymous>:14477:55)
    at instantiate (<anonymous>:12118:23)
    at lookup (<anonymous>:11940:17)
    at Object.Container.lookup (<anonymous>:11856:14)
    at didTransition (<anonymous>:183:39)
    at Object.triggerEvent (<anonymous>:38984:35)
    at Object.trigger (<anonymous>:63444:14)


undefined:14270
      throw new Error("DOM Helper could not find valid URL parsing mechanism")
            ^

The error is raising here and it looks like module.require is undefined. I'm not sure if there's a missing/wrongly versioned dependency or if something else is amiss.

tomdale · 2015-12-10T15:49:23Z

@ronco Ah, the problem is that the sandboxed Ember app does not have access to module.require. We could add it to the sandbox but it does mean that the JavaScript code will have unlimited access to the filesystem among other things. This is related to https://gist.github.com/tomdale/239676ad88289c56b74e.

tomdale · 2015-12-10T15:49:51Z

@ronco The Ember tests didn't catch this because they are not running sandboxed like FastBoot apps are.

tomdale · 2015-12-10T15:50:00Z

cc @chancancode

ronco · 2015-12-10T15:54:33Z

@tomdale Yeah, I thought it might be a sandboxing issue. Let me know if there's anything I can do to help.

tomdale · 2015-12-10T17:10:46Z

@ronco Can you patch https://github.com/tildeio/htmlbars/blob/master/packages/dom-helper/lib/main.js#L571-L598 to use the global URL and only require it if it's not available? I see now that we're explicitly making the URL package available in the FastBoot sandbox: https://github.com/ember-fastboot/ember-fastboot-server/blob/master/lib/ember-app.js#L81

ronco · 2015-12-10T17:36:00Z

@tomdale On it.

ronco · 2015-12-10T17:55:31Z

@tomdale tildeio/htmlbars#439

ronco · 2015-12-14T05:43:07Z

@tomdale I started putting together an addon for head.hbs here. It works great with a browser and I plan on updating my meta tags addon to be built on top of it.

However, when I try to integrate it with fastboot I'm running into some issues. I have a demo app I was testing it with here (using npm link to pull in the unpublished addon).

Running fastboot with the addon results in:

ReferenceError: document is not defined
    at new DOMHelper (<anonymous>:13840:34)
    at DOMHelper.EmberDOMHelper (<anonymous>:22948:24)
    at _emberRuntimeSystemObject.default.extend.init (<anonymous>:55778:69)
    at superWrapper (<anonymous>:33700:22)
    at exports.default._emberMetalMixin.Mixin.create.init (<anonymous>:52760:19)
    at superWrapper (<anonymous>:33700:22)
    at exports.default._emberMetalMixin.Mixin.create.init (<anonymous>:52099:19)
    at superWrapper (<anonymous>:33700:22)
    at exports.default._emberMetalMixin.Mixin.create._Mixin$create.init (<anonymous>:53565:19)
    at superWrapper (<anonymous>:33700:22)
500 Unknown Error: ReferenceError: document is not defined

The issue arises when looking up the head component from the container in my instance-initializer. This ends up triggering this code path in CoreView. renderer:-dom is registered and has the correct fastboot simpledom document, however that's not provided to the component at create time in this flow.

I was trying to come up with a way to hack it in there within the addon, but I'm wondering if I'm shaving the wrong yak there.

I'd appreciate any thoughts suggestions on how to get this working.

tomdale · 2016-02-10T19:50:08Z

Closed in favor of #114

najax@0.6.0 for gzip support

…mocha-4.0.0 Update mocha to the latest version 🚀

…tadata

jpadilla and others added 5 commits August 12, 2015 18:29

Implement naive meta tag support

943c6eb

Add meta tags using document.head

2739d5d

This assumes we're able to merge ember-fastboot/simple-dom#12

Update simple-dom to 0.2.7

4948e07

Simple serialization of simple-dom head

050fc28

- just serialize the head tag from the document directly - leave population of the tag to application code

Merge branch 'master' into meta-tags

717c784

ronco mentioned this pull request Sep 23, 2015

replace HEAD placeholder (rather than just title) ember-fastboot/fastboot#1

Closed

ronco mentioned this pull request Oct 2, 2015

expose serializer method for child nodes ember-fastboot/simple-dom#18

Merged

use serializeChildren for head & body

42cf561

rwjblue mentioned this pull request Dec 7, 2015

Support Meta Tags (OpenGraph / Twitter Cards) #77

Closed

ronco mentioned this pull request Feb 10, 2016

Add a placeholder comment for generic head #114

Merged

tomdale closed this Feb 10, 2016

arjansingh pushed a commit to arjansingh/ember-cli-fastboot that referenced this pull request Oct 29, 2016

Merge pull request ember-fastboot#66 from pwfisher/najax-gzip

47343b5

najax@0.6.0 for gzip support

xg-wang pushed a commit to xg-wang/ember-cli-fastboot that referenced this pull request Nov 16, 2020

Merge pull request ember-fastboot#66 from ember-fastboot/greenkeeper/…

7d91a54

…mocha-4.0.0 Update mocha to the latest version 🚀

xg-wang pushed a commit to xg-wang/ember-cli-fastboot that referenced this pull request Nov 28, 2020

Merge pull request ember-fastboot#66 from suchitadoshi1987/suchita/me…

6289968

…tadata

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow Populating Head Content #66

Allow Populating Head Content #66

ronco commented Sep 23, 2015

ronco commented Sep 23, 2015

rwjblue commented Sep 23, 2015

jpadilla commented Sep 23, 2015

ronco commented Sep 23, 2015

ronco commented Sep 25, 2015

rwjblue commented Sep 30, 2015

ronco commented Sep 30, 2015

rwjblue commented Oct 2, 2015

ronco commented Oct 2, 2015

rwjblue commented Oct 2, 2015

chancancode commented Oct 2, 2015

ronco commented Dec 9, 2015

rwjblue commented Dec 9, 2015

tomdale commented Dec 9, 2015

ronco commented Dec 9, 2015

tomdale commented Dec 9, 2015

ronco commented Dec 9, 2015

ronco commented Dec 10, 2015

tomdale commented Dec 10, 2015

tomdale commented Dec 10, 2015

tomdale commented Dec 10, 2015

ronco commented Dec 10, 2015

tomdale commented Dec 10, 2015

ronco commented Dec 10, 2015

ronco commented Dec 10, 2015

ronco commented Dec 14, 2015

tomdale commented Feb 10, 2016

Allow Populating Head Content #66

Allow Populating Head Content #66

Conversation

ronco commented Sep 23, 2015

ronco commented Sep 23, 2015

rwjblue commented Sep 23, 2015

jpadilla commented Sep 23, 2015

ronco commented Sep 23, 2015

ronco commented Sep 25, 2015

rwjblue commented Sep 30, 2015

ronco commented Sep 30, 2015

rwjblue commented Oct 2, 2015

ronco commented Oct 2, 2015

rwjblue commented Oct 2, 2015

chancancode commented Oct 2, 2015

ronco commented Dec 9, 2015

rwjblue commented Dec 9, 2015

tomdale commented Dec 9, 2015

ronco commented Dec 9, 2015

tomdale commented Dec 9, 2015

Example

ronco commented Dec 9, 2015

ronco commented Dec 10, 2015

tomdale commented Dec 10, 2015

tomdale commented Dec 10, 2015

tomdale commented Dec 10, 2015

ronco commented Dec 10, 2015

tomdale commented Dec 10, 2015

ronco commented Dec 10, 2015

ronco commented Dec 10, 2015

ronco commented Dec 14, 2015

tomdale commented Feb 10, 2016