kadirahq / fast-render

Render you app even before the DDP connection is live. - magic?
MIT License
560 stars 80 forks source link

Fast Render Not Working in Meteor 1.4.2 #186

Closed simsim0709 closed 7 years ago

simsim0709 commented 7 years ago

I recently update to meteor 1.4.2, and fast-render is not injecting data into html. Is it right? or am I missing something?

screen shot 2016-10-26 at 4 03 40 pm

oychao commented 7 years ago

OH MY GOSH, I wasted a whole afternoon just because I updated Meteor? .......

devwax commented 7 years ago

Same here... at least I know what the problem is now.

Haven't tried downgrading yet.

devwax commented 7 years ago

Update: I've tried installing older meteor versions back to 1.3.5.1 to no avail. Still doesn't work. What versions have others used that are known to work?

devwax commented 7 years ago

Update 2: (partial success) I created a fresh test project with meteor 1.3.5.1 w/ the bare minimum for meteor package: reactrouter:react-router-ssr which uses fastrender and it works now. The rehydrateHook() receives the data and it's present in the js inject code in the client-side source.

meteor create frtest --release 1.3.5.1

Will try updating and adding packages to pinpoint further.

oychao commented 7 years ago

I'm currently using 1.4.2, It seems all versions after 1.4 don't work, anyone succeeded~?

devwax commented 7 years ago

Update 3: Ok, gets weirder... I upgraded my test installation that was at 1.3.5.1 to 1.4

meteor create frtest --release 1.4

and it continued to work.

Then I tried adding some packages, removing insecure and autopublish to rule those out and it still worked.

Then I tried updating to the newest meteor version 1.4.2 and it stopped working, as expected.

But when I tried to revert back to 1.4 with meteor create frtest --release 1.4, now fast render no longer works when it worked at that version before.

So updating to >1.4 somehow changes your installation so that even if you revert back to a known working version of meteor, fastrender still wont work.

Maybe it's node or the node version? FastRender injects data into the server response in webapp middleware (var res). Maybe meteor upgrading to 4 from 0.xx.x has something to do with it?

Next, I'm going to try creating a fresh project with the latest meteor version and see if that works. [Update: tried it, doesn't work]

And if that doesn't work I'm going to start fresh at the last known working install (1.4) and incrementally upgrade meteor until I find the offending version. [Update: that doesn't work either... only works if you start at 1.3.5.1 and upgrade to 1.4.0. A fresh install of >=1.4.0 doesn't fix the issue..]

Will report back and possibly open and issue at meteor/meteor if necessary.

devwax commented 7 years ago

Update 4: I'm pretty sure it's the webapp package >1.3.10. I think that's the reason why fastrender continued to work when I upgraded from 1.3.5.1 to 1.4.0.

When you upgrade to 1.4 from 1.3 there are a number of packages that don't get updated automatically. You need to explicitly run meteor update --packages-only to update them. I hadn't done that before I tested it again, so webapp wasn't updated.

And when I did update the packages and webapp updated from 1.3.10 to 1.3.12 that's when fastrender stoped working.

I'll continue investigating and submit a bug report @ meteor/meteor when I have more.

Update: Issue opened at meteor/meteor #7992

simsim0709 commented 7 years ago

What I found until now: I think the main problem of this issue is InjectData package. This package tries to override res.write method. In that method, there is conditional statement below.

var condition =
      res._injectPayload && !res._injected &&
      encoding === undefined &&
      /<!DOCTYPE html>/.test(chunk);

These conditions are true except last one. /<!DOCTYPE html>/.test(chunk) this condition always fail, because chuck parameter is not string but object(buffer) always.

So, I downgraded webapp package to 1.3.10(@rigconfig advised), and conditional statement return true. After that final string is what i expect like this, but weird thing is that in the browser, result html is same as before.

Although, I downgraded webapp to 1.3.10. It doesn't solve the issue anyway.

devwax commented 7 years ago

@simsim0709 I've got an update here at Meteor's issue tracker that addresses data-inject package. I just commented out a few lines above that to get it working again.

I've forked the inject-data repo w/ the fix

cd packages/
git clone https://github.com/rigconfig/inject-data
meteor run
simsim0709 commented 7 years ago

@rigconfig Thanks. It's working fine! You did good job!! :)

abecks commented 7 years ago

Hey @rigconfig, trying to use your fix but still not receiving a FastRender payload.

I cloned your repo to /packages/ and ran meteor add meteorhacks:inject-data. Do I need to downgrade webapp to 1.3.10 as well or should it be working now?

devwax commented 7 years ago

@abecks Actually, you don't need to manually add meteorhacks:inject-data. It's a dependency of the fast-render package. I'm thinking that maybe because you manually added inject-data package it's not using the local package.

Try removing or commenting out the meteorhacks:inject-data line in your ./meteor/packages file, restart and let me know if that changes anything.

abecks commented 7 years ago

Hey @rigconfig,

Unfortunately that didn't work either. Are you running the fix with webapp@1.3.12 or 1.3.10? I've been trying to downgrade to 1.3.10, but its a bit of a rabbit hole dependency wise (mongo, accounts-base, and service-configuration all have to be downgraded too). Maybe you know of an easier way to downgrade?

devwax commented 7 years ago

@abecks Oh, you don't need to downgrade webapp... I'm using it with the latest webapp@1.3.12 and the latest Meteor version 1.4.2.

Can you give me a list of your packages and meteor version? Maybe I can reproduce it.

abecks commented 7 years ago

Appreciate the help.

Ok, project starts out as 1.4.1.1.

I run meteor update to get to 1.4.2.

I run:

cd packages/
git clone https://github.com/rigconfig/inject-data

and launch the app. I log in, then refresh the page, expecting FastRender to do its magic. FastRender.debugger.getPayload returns undefined.

I added a console log to the condition check in inject-data/lib/server.js:

  res.write = function(chunk, encoding) {
    var condition =
      res._injectPayload && !res._injected &&
      encoding === undefined &&
      /<!DOCTYPE html>/.test(chunk);

    console.log('condition', condition);

condition is always false. It looks like what @simsim0709 was referring to where the chunk is now a buffer and no longer a string. If its working for you on webapp@1.3.12, how is your condition evaluating to true?

Here is my full package list:

# Meteor packages used by this project, one per line.
# Check this file (and the other files in this directory) into your repository.
#
# 'meteor add' and 'meteor remove' will edit this file for you,
# but you can also edit it by hand.

meteor-base@1.0.4             # Packages every Meteor app needs to have
mobile-experience@1.0.4       # Packages for a great mobile UX
mongo@1.1.14                   # The database Meteor supports right now
blaze-html-templates@1.0.4    # Compile .html files into Meteor Blaze views
reactive-var@1.0.11            # Reactive variable for tracker
jquery@1.11.10                  # Helpful client-side library
tracker@1.1.1                 # Meteor's client-side reactive programming library

standard-minifier-js@1.2.1    # JS minifier run for production mode
es5-shim@4.6.15                # ECMAScript 5 compatibility for older browsers.
ecmascript@0.5.9              # Enable ECMAScript2015+ syntax in app code
aldeed:simple-schema
kadira:flow-router
kadira:blaze-layout
accounts-base@1.2.14
accounts-password@1.3.1
less@2.7.6
aldeed:collection2
aldeed:autoform
staringatlights:autoform-generic-error
arillo:flow-router-helpers
jwo1f:parent-template
meteorhacks:fast-render
random@1.0.10
alanning:roles
tmeasday:acceptance-test-driver
xolvio:cleaner
reactive-dict@1.1.8
juliancwirko:s-alert
juliancwirko:s-alert-flip
session@1.1.7
dispatch:phoneformat.js
copleykj:livestamp
meteorhacks:subs-manager
aldeed:template-extension
tinymce
agnito:autogrow
email@1.1.18
http@1.2.10
meteortypescript:typescript-libs
fortawesome:fontawesome
perak:codemirror
seba:minifiers-autoprefixer
meteorhacks:aggregate
okgrow:router-autoscroll
meteorhacks:picker
matb33:collection-hooks
meteorhacks:kadira
tomwasd:history-polyfill
meteorhacks:zones
shell-server@0.2.1
reywood:publish-composite
muqube:autoform-nouislider
facts@1.0.9
devwax commented 7 years ago

If its working for you on webapp@1.3.12, how is your condition evaluating to true?

I haven't checked the condition that @simsim0709 was referring to. I just commented out the if statement at the beginning of the function to fix it.

I'll check the condition in my working version and then will try with your packages to see if there's a conflict there. Will post back. I'm on the road at the moment.

devwax commented 7 years ago

@abecks Ok, quick question... How are you achieving server-side rendering? I was looking through your packages and didn't see anything that does ssr? I'm probably missing something obvious, because I don't use blaze or flow-router anymore (switched to react and meteor-react-router-ssr a while ago).

From what I remember, to do ssr you need to install the package kadira:flow-router-ssr. Also, it says in the docs that it only works w/ react.

But again, I haven't been using flow router or blaze for a while, so I may be missing something simple?

If you could check your .meteor/versions file to see if inject-data is there or just post the list here.

abecks commented 7 years ago

Hey @rigconfig, I'm not server side rendering actually. But I prefer to have FastRender delivering the user's authentication status with the initial payload (which it should do on its own). I'm currently using FlowRouter.

devwax commented 7 years ago

@abecks Ok, thanks for the clarification. I'll see what I can do with creating a collection and subscription w/ your package list and see if I can duplicate the problem.

It might be easier though if you could create a reproduction recipe:

  • Create a new Meteor app that displays the bug with as little code as possible.
  • Create a new GitHub repository with a name like meteor-reactivity-bug [...] and push your code to it.
  • Reproduce the bug from scratch, starting with a git clone command.
devwax commented 7 years ago

Yeah, I'm not familiar enough with the technique you're employing, so I would need to see a minimal reproduction of the issue as mentioned above. I'l certainly take a look, though, if you want to do that.

abecks commented 7 years ago

Heres what I am talking about: https://meteorhacks.com/instant-login-for-meteor-with-fast-render/

Fast-Render looks for all the null publications and sends all their documents to the client with the initial HTML.

Because Fast Render is not sending any payload for me on 1.4.2, I am not receiving the "instant login" functionality that Fast Render provides. My FlowRouter routes trigger before the user is logged in, so every page refresh the user gets redirected to the login page, even though they are logged in.

I'll try to piece together a reproduction repo for you. Thanks for your time on this.

anschutz commented 7 years ago

Having exactly the same thing as @abecks, since updating to 1.4.2 fast render stopped working completely. I also use it for "instant login". Seems that chunk is always a buffer indeed.

abecks commented 7 years ago

I remember reading a comment by @arunoda somewhere that they needed to switch FastRender to use a new Meteor API for injecting data (instead of his inject-data package). I can't remember off the top of my head where I saw that comment though.

I know @arunoda/kadira have abandoned these Meteor repositories, so someone will have to fork FastRender to update it to the new injection API.

abecks commented 7 years ago

Hey @anschutz, just wondering if you found a solution yet. If not, I'll try to take a stab at forking Fast Render.

anschutz commented 7 years ago

Hi @abecks, looks like the reason Fast Render can't inject the data is described in meteor/meteor#7753. I just had to roll back to Meteor 1.4.1.3 for now.

There's an update in meteor/meteor#7992 where they suggest to use dynamicBody or dynamicHead instead of intercepting server messages.

adrianhunter commented 7 years ago

@abecks if you only need it for instant login, you could simply "fake" the InjectData approach with dynamicHead

with this code you can still get your instant login:

WebApp.connectHandlers.use((req, res, next)->
    if not IsAppUrl(req)
        next()
        return

    loginToken = req.cookies['meteor_login_token']

    if loginToken
        loginToken = Accounts._hashLoginToken loginToken

        user = [Meteor.users.find({'services.resume.loginTokens.hashedToken': loginToken}).fetch()]

    payload = {
        subscriptions: {users: true},
        collectionData: {
            users: user
        }
    }

    data = {
        'fast-render-data': payload
    }

    req.dynamicHead = "<script type='text/inject-data'>#{encodeURIComponent (EJSON.stringify data)}</script>"
    next()
)
abecks commented 7 years ago

@adrianhunter That is wicked. Thanks for the example. I still need FastRender's subscriptions as well, so I'm going to attempt a PR fix for this.

abecks commented 7 years ago

I have added support for Meteor 1.4.2+ via the new dynamicHead feature in this PR to meteorhacks:inject-data.