meteorhacks / meteor-inject-initial

Allow injection of arbitrary data to initial Meteor HTML page
Other
78 stars 11 forks source link

Broken since `1.3-modules-beta.7` #18

Closed trusktr closed 8 years ago

trusktr commented 8 years ago

I'm trying

Inject.rawModHtml('raf-timeout', function(html, res) {
    console.log(' ------ HTML', html)
})

on the server side, but that console.log never fires when I refresh the client.

trusktr commented 8 years ago

Oh, I'm in Meteor 1.3-beta.11, maybe that's why?

gadicc commented 8 years ago

Yes, confirming that this doesn't work in at least 1.3-beta.11.

What's your use case btw? If you don't actually need all the power of rawModHtml, and just need to insert some code into the head or body, https://github.com/meteor/meteor/pull/3860 just got merged and should be in the next release, and should be considered the official pattern moving forward.

Will try figure out what's going on when I get a chance.

gadicc commented 8 years ago

Probably relates to https://github.com/meteorhacks/inject-data/commit/1f7e84f0574f2ab2d34138a366c0c4f834059475

ghost commented 8 years ago

@gadicc My use case is I just want to load raf-timeout before all code, to see how it affects performance. What do you think of that concept? I'm also wondering if there's other ways to force code (f.e. 3rd party libraries out of our control) to run everything in an rAF. What if we load all Meteor APIs and the app bundle in an initial rAF, and override all things in the browser that could possibly defer to a future tick so that those things also run in a future rAF, not just any future tick? For example, what about monkey patching EventTarget.prototype.addEventListener to use rAF? The idea behind these modifications is that people would be able to use existing browser APIs naively, but with better graphical performance than before. For example, someone could do

setInterval(function() {...}, 16)

for an animation loop, and for the most part their code would still benefit from rAF. Essentially, the event loop would be limited to the frames between paints. In most cases, 16ms is not perceivable so letting an event handler or timeout wait that long is just fine.

Here, for example, MDN suggests using rAF for scroll events.

The question is, when do we actually need to run event handlers, timeouts, intervals, promise resolutions, etc, faster than every 16ms, and can we survive without that ability? Maybe setTimeout can be smart, and if inside an rAF then setTimeout can behave like normal (but how would we know when the end of an rAF is coming)?

Just some thoughts. :} You asked for my use case. xD Those are some ideas I was thinking about yesterday.

ghost commented 8 years ago

I'm imagining, that await something would cause the code that follows it not to fire until at least the next rAF. I don't see any need for code to run outside of rAF (unless it's a crypto algorithm or similar, and we know there's no animation happening at the same time, etc). For the average app, limiting all code to being inside rAF seems like it would be fine...

gadicc commented 8 years ago

Regarding script loading

You could do this with the new (Meteor) API, since script loading has been moved to the BODY, and you can insert this into the HEAD.

Regarding the philosophy

In theory you'd be offering developers a "magical" easy way to improve performance, which is quite nice by default. I think though for truly optimized and performant code you'd want to give developers the choice of where to run what. I'd love to see the results of this.

ghost commented 8 years ago

@gadicc I posted some ideas in raf-timeout about what can defer.

gadicc commented 8 years ago

@trusktr-goin, great! Will respond when I have a chance. Meanwhile, this issue is fixed in v1.0.4-rc.0.

@arunoda, all look ok? It's the same strategy you used in inject-data, except we don't need to "checkIfNeeded", since, I had to add a connectHandler anyway to accommodate the parts of the API that don't require a "res". All tests are passing (in old and new Meteor); had to modify the injection test of course.