search

schultek / jaspr

Modern web framework for building websites in Dart. Supports SPAs and SSR.
https://jasprpad.schultek.de
MIT License
996 stars 59 forks source link

Jaspr version 0.11.1 Fails to load content correctly when using a custom index.html file #217

Closed ponderMuse closed 2 months ago

ponderMuse commented 3 months ago

Description

Running a simple SSR project that uses a custom index,html file on initial load fails to load js content when run using Jaspr version 0.11.1 (Custom index.html files worked fine in previous Jaspr version 0.10.0).

Steps To Reproduce

Clone project

  1. Ensure Jaspr is updated to version 0.11.1
  2. Clone example minimal SSR project at https://github.com/ponderMuse/jaspr_index_html_issue
  3. jaspr clean
  4. jaspr build
  5. jaspr serve

Doctor Output

[✓] Jaspr CLI (Version 0.11.1) • Dart Version 3.3.0 (stable) (Tue Feb 13 10:25:19 2024 +0000) on "linux_x64" at /mnt/data/flutter/bin/cache/dart-sdk/bin/dart • Running on linux Linux 5.15.0-97-generic #107-Ubuntu SMP Wed Feb 7 13:26:48 UTC 2024 - Locale en_GB.UTF-8 • Analytics: Enabled

[✓] Current Project • Dependencies on core packages: • jaspr: ^0.11.1 • jaspr_builder: ^0.11.1 (dev) • jaspr_router: ^0.4.0 • Rendering mode: server • Uses jaspr compilers: false • Uses flutter embedding: false

Expected Behavior

App should initially load index.html body content and once the client.dart.js script has been loaded the content should be replaced with the actual client.dart.js contents.

Additional Context

In theory, an index.html file should not be required for a Jaspr app as Jaspr provides a Document class with necessary head links, metas, etc.) that one can replace an index.html file with but, as far as I can see, there are a couple of omissions in the Document class that can hurt PageSpeed Insight measurement metrics (important for SEO).

The things that I believe are missing in Document class are:

I have tried to set the initial <body> content by doing for example:

runApp(Document(body: body([img(...),App()])));

But it doesn't seem to behave in quite the same way as having the img in an actual index.html file does.

schultek commented 2 months ago

I've done some digging and fixing Document.file is not an easy task. The problem is that the server never knows if a static file should be served as-is or if it needs to run the server rendering code which potentially consumes this file. Right now it always serves a file as-is which results in seeing the un-modified index.html file. This was an oversight in the latest version but I also don't know how to effeciently solve this because it cannot be statically known if Document.file is used or not.

However Document.file will stay a second class citizen in Jaspr. As you pointed out In theory, an index.html file should not be required for a Jaspr app. This is not just in theory. You can (and I would recommend) just write jaspr code instead of using an index.html. I even thought about removing Document.file entirely, and still might do so if I don't find a good solution for this problem.

but, as far as I can see, there are a couple of omissions in the Document class that can hurt PageSpeed Insight measurement metrics (important for SEO). The things that I believe are missing in Document class are:

lang attribute.

Yes thats not a separate option right now, but you can add it yourself using the head parameter.

A way of providing content for the that gets shown while the client.dart.js script is being fetched.

Thats already working. However runApp(Document(body: body([img(...),App()]))); is wrong because Documentalready adds a body tag, so this would result in <html><body><body><img/></body></body></html>. Use a separate stateless component or the Builder if you want to have multiple children in body.

ponderMuse commented 2 months ago

I have already updated my app to build the root document programmatically instead of using an index.html and it works great but I just cannot get the app to produce as high a score on PageSpeed Insights as I could using an actual index.html with Jaspr 0.10.0.

I don't want multiple children in the body at all. body([img(...),App()]) was just a poor attempt by me to try and get an image placed inside <body> while the js script is fetched. What I am after is just for a preload image to be shown in <body> until the js script has been fetched and at which point, the image in <body> gets replaced altogether by the js script's content thus, resulting in a high score for PageSpeed's FCP and LCP (specifically on mobile).

Is there a code example I can look at somewhere that does: 'Use a separate stateless component or the Builder...'?

Also, are there any live Jaspr 0.11.1 sites I can look at that do score green on all fronts in PageSpeed Insights mobile metrics? I must clearly be missing something somewhere if I'm the only one struggling to reach a +90% green metric for FCP/LCP on mobiles using a programmatic approach to build the site's root Document.

schultek commented 2 months ago

I don't really get the whole thing with the image. When you use server-side rendering, you already have the whole page content available before the js is loaded. Why show and replace an image then?

ponderMuse commented 2 months ago

Yes, I agree. This app is an SSR but for some reason it just does not score the same mobile metrics when built using Jaspr 0.11.1 as it did when built using Jaspr 0.10.0 with an index.html file for the root Document.

I must clearly be missing something somewhere. Leave it with me and I will take a deeper look to see if I can spot what's different in my 0.11.1 build compared to the previous 0.10.0 build.

schultek commented 2 months ago

I think I know a way to fix this:

We differentiate between normal '.html' files and '.template.html' files, where html files are served as-is and template files are only used together with Document.file, which also cannot use normal html files.

@ponderMuse Wdyt?

schultek commented 2 months ago

Can you try if the implementation works for you using the following dependency override:

dependency_overrides:
  jaspr:
    git:
      url: https://github.com/schultek/jaspr
      path: packages/jaspr
      ref: improvement/document-template
ponderMuse commented 2 months ago

I'll update the project to include your improvement/document-template git ref and see how it goes.

By the way, only spend time on this issue if it makes sense to you and the project in general. Like I said, if it is just me experiencing this behaviour with 0.11.1 then it is way more likely that it is just me doing something silly with the way I am coding the root document and SSR code in general more than a problem with 0.11.1 itself. The PageSpeed Insights metrics I get with 0.11.1 are only worst for the FCP/LCP part (green 92% using 0.10.0 and yellow %86 when using 0.11.1) but the overall SEO score is still green and good enough with either 0.10.0 and '0.11.1'.

ponderMuse commented 2 months ago

So I've updated the project to use the improvement/document-template git ref and when I run it, the index.html is loaded from file but the <body> content doesn't get replaced with the client.dart.js contents once the js file has been fetched.

With this improvement/document-template git ref I am having to run the app in main() like this runApp(Document(body: App())); where the app seems to pick up index.html indirectly. With previous 0.10.0 version I was running the app in main() like this runApp(Document.file(name: 'index.html', child: App()));. In 0.11.1 the static call Document.file() isn't available any more.

Like I said in previous post, don't go out of your way trying to get this resolved if others are not experiencing similar problems. 0.11.1 as it is still produces good high SEO score overall so it is not a showstopper in any way.

schultek commented 2 months ago

Please check the changelog on how to use Document.template: https://github.com/schultek/jaspr/blob/22ff349155067ff7836326ad36b5fea83b2c3f18/packages/jaspr/CHANGELOG.md

By the way, only spend time on this issue if it makes sense to you and the project in general. Like I said, if it is just me experiencing this behaviour with 0.11.1 then it is way more likely that it is just me doing something silly with the way I am coding the root document and SSR code in general more than a problem with 0.11.1 itself.

Thanks but don't worry, the index not loading correctly is a real problem that needs fixing.

I'm not doing anything on the seo insights right now as I believe this really is just a configuration / usage problem.

ponderMuse commented 2 months ago

Please check the changelog on how to use Document.template: https://github.com/schultek/jaspr/blob/22ff349155067ff7836326ad36b5fea83b2c3f18/packages/jaspr/CHANGELOG.md

Have just read and updated the app using improvement/document-template git ref to use Document.template() instead and now the app launches okay with the file named using form index.template.html. When I load different linked pages (@client components) however, some pages are failing to load but not others. The pages that do fail are showing the following error on the client console:

Error on rebuilding component: Assertion failed: org-dartlang-app:///packages/jaspr/src/browser/dom_render_object.dart:209:14 parentNode is html.Element is not true Uncaught Error: Assertion failed: org-dartlang-app:///packages/jaspr/src/browser/dom_renderobject.dart:209:14 parentNode is html.Element is not true at Object.throw [as throw] (errors.dart:297:3) at Object.assertFailed (errors.dart:38:3) at dom_render_object.DomRenderObject.new.attach (dom_render_object.dart:209:24) at framework.DomElement.new.attachRenderObject (render_object.dart:77:5) at render_object.dart:60:7 at framework.dart:810:18 at framework.BuildOwner.new.performRebuildOn (build_owner.dart:86:7) at framework.DomElement.new.rebuild (framework.dart:793:5) at [_firstBuild] (multi_child_element.dart:37:5) at [_firstBuild] (render_object.dart:59:11) at framework.DomElement.new.mount (multi_child_element.dart:31:5) at framework.StatefulElement.new.inflateComponent (framework.dart:448:13) at framework.StatefulElement.new.updateChild (framework.dart:314:18) at framework.StatefulElement.new.updateChildren (multi_child_element.dart:197:32) at framework.StatefulElement.new.performRebuild (multi_child_element.dart:69:17) at framework.StatefulElement.new.performRebuild (stateful_component.dart:680:11) at framework.BuildOwner.new.performRebuildOn (build_owner.dart:85:12) at framework.StatefulElement.new.rebuild (framework.dart:793:5) at framework.BuildOwner.new.performBuild (build_owner.dart:146:18) at [_handleFrame] (scheduler.dart:19:5) at scheduler.dart:14:25 at browser_binding.dart:95:7

Thanks but don't worry, the index not loading correctly is a real problem that needs fixing. Understood.

I'm not doing anything on the seo insights right now as I believe this really is just a configuration / usage problem. Agree 100%.

ponderMuse commented 2 months ago

To add to the console errors shown, the pages that fail to load are the pages which are StatefulComponents. All other pages are StatelessComponents and they load and display okay.

schultek commented 2 months ago

When I load different linked pages (@client components) however, some pages are failing to load but not others. The pages that do fail are showing the following error on the client console

@ponderMuse Can you open a new issue for that, ideally with another reproducible example? I don't think its related.

I will close this one then.

ponderMuse commented 2 months ago

When I load different linked pages (@client components) however, some pages are failing to load but not others. The pages that do fail are showing the following error on the client console

@ponderMuse Can you open a new issue for that, ideally with another reproducible example? I don't think its related.

I will close this one then.

Hey, sure. Will try and get a minimal example going that produces the parentNode is html.Element is not true error and raise a ticket for it (Unless I figure it is something at my end that was causing it).