Avoid fails from serverside renderings #3909
Conversation
khorolets
force-pushed
the
avoid_serverside_rendering_fails
branch
4 times, most recently
from
5c20ce9 to
c29003e
Compare
| @@ -669,6 +669,11 @@ module.exports = function(Chart) { | |||
| }; | |||
| // Request animation polyfill - http://www.paulirish.com/2011/requestanimationframe-for-smart-animating/ | |||
| helpers.requestAnimFrame = (function() { | |||
| if (typeof window === 'undefined') { | |||
There was a problem hiding this comment.
I'm not sure how I feel about this change. I understand that it's rendering the chart synchronously if window is not defined, but that can be achieved by setting the animation duration to 0. @simonbrunel thoughts?
There was a problem hiding this comment.
I think this use case (server side rendering) should rely on a new platform (e.g. platform.node) but not sure how it would be implemented. For now it may be easier to merge these (harmless) changes if it helps projects to integrate Chart.js.
@khorolets are these the only checks needed in order to have Chart.js running server side? What about the other specific HTMLCanvasElement, CanvasRenderingContext2D, CanvasGradient or even the iframe?
There was a problem hiding this comment.
We (@khorolets and I) don't use Chart.js to render charts on the server, that being said, we wrap the code calling Chart.js rendering into guards to avoid the execution on the server. HOWEVER, without this patch, we cannot have Chart.js in our bundle because Chart.js initializes some of its parts right away.
There was a problem hiding this comment.
What about the other specific
HTMLCanvasElement,CanvasRenderingContext2D,CanvasGradientor even theiframe?
Since we don't encounter any other problems, I believe those parts are not triggered on script evaluation, and that is fine with our server side.
There was a problem hiding this comment.
Makes sense, can you rebase and resolve conflicts?
There was a problem hiding this comment.
@simonbrunel I have already asked @khorolets to do that in a private chat, but I believe he is already offline for today, so we will address this the first thing tomorrow morning.
There was a problem hiding this comment.
@etimberg are you good to merge those changes? We can still plan a better handling of window by moving all references into the dom platform.
There was a problem hiding this comment.
@simonbrunel I've resolved the conflicts.
khorolets
force-pushed
the
avoid_serverside_rendering_fails
branch
from
c29003e to
4961223
Compare
|
When will this be pushed into a release? Desperately need this. |
|
Also need this |
|
@AlexKvazos @trevordmiller we're working to get a 2.6 release out soon (it kinda got a bit bigger than anticipated and @simonbrunel and I have gotten busy). I think once we have docs deployed we should be good to go. I'm hoping in the next week or so |
|
@etimberg Thank you, that will be very helpful to have this. |
Hi!
I'm using Chart.js in my project and I also use server-side rendering. And if my page contains Chart.js rendering is failing (expectedly). I tried to wrap usage of Chart.js with something like:
But it still fails because Chart.js has been imported before and has tried to use
windowalready.I came up with the fix in PR, please take a look.
Here are some links to issues with the problem similar to mine:
P.S. Thanks for your great work!