Closed jjrchrds closed 3 years ago
We just published v2.2.8, which waits for window load (rather than component mount) to calculate offset heights and intersection margins (4d6a52b3f7d497ceda6a84c3806de46c8e527474). Could you try updating your React Scrollama and seeing if the bug persists?
Hi there! first of all thanks for the lib. I really like the simplicity of the api. I ran into an issue that is related to the fix you are mentioning for this issue.
Since you are now "calculate(ing) offset heights and intersection margins" only on document.load and not on component mount <Scrollama />
is only usable in true SPA's. When you mount <Scrollama />
after the document was loaded you have no way of using the lib anymore. This is easily the case when e.g. you have a next.js application and navigate from one page to another.
Could you elaborate why this change was introduced? To me it feels counter intuitive to trigger the calculation only on docoument.load. I believe you actually should calculate anything on componentDidMount
rather explicitly triggering the whole lib on document.load.
Ran into same issue @maerzhase is describing, tried loading via dynamic import in next.js with ssr:false option, on document.load it works, and then on route change stops working
Ah, thanks so much for the explanations @maerzhase @olegrjumin. Apologies, I've never really worked with non SPAs before. Would a solution be to move the calculations back to componentDidMount
but also add a calculation trigger on document.load
for redundancy? The reason why I had initially moved the calculations to on-page-load in the first place was that the library needs to access DOM element offsets and heights, but sometimes those properties aren't accurate/available yet at componentDidMount
. I'm not sure if this is a universal React thing, or there's some sort of race condition between different component renders that create this inaccuracy.
no luck for me still.
this works
` componentDidMount() { const scrollama = require("scrollama") const scrollThreshold = 0.33 this.scroller = scrollama()
this.scroller
.setup({
container: "#narrative-scroll",
step: ".narrative-step",
threshold: scrollThreshold,
progress: true,
debug: false,
})
.onStepEnter(this.handleScrollStepEnter)
.onStepExit(this.handleScrollStepExit)
.onStepProgress(this.handleProgress)
// setup resize event
window.addEventListener("resize", this.scroller.resize)
}`
From the react documentation doing the whole calculation in componentDidMount
should be the exact place to do so:
https://reactjs.org/docs/react-component.html#componentdidmount
The DOM is supposed to be available. Could you share an example were you ran into the problem that you could not retrieve DOM elements on mount? Maybe this was in a early version of react?
Same goes for the original issue here. @jjrchrds could you share an runnable environment that shows your issue? From what I undestand going from componentDidMount
to document.load
never actually solved the original issue...
@maerzhase thanks for your fix, It unblocked me!
@maerzhase I may have spoken too soon. My website runs with gatsby develop
, but will not build (gatsby build
). Are you able to build your site using your forked repo?
Looks like the lib is trying to reference window at build time:
"window" is not available during server side rendering.
Jeah this will always be the case you have to opt out server side rendering for this lib, since there is no window on the server. Usually you do this with dynamic imports. e.g. https://nextjs.org/docs/advanced-features/dynamic-import
@maerzhase thanks for the suggestion. I tried out an implementation using loadable-components:
import loadable from '@loadable/component'
const Scrollama = loadable(() => import('~utilities/react-scrollama'), {
resolveComponent: components => components.Scrollama,
})
const Step = loadable(() => import('~utilities/react-scrollama'), {
resolveComponent: components => components.Step,
})
But now I'm getting a new error when trying to execute the built bundle:
Could not get step with id react-scrollama-0 react-dom.production.min.js:209
I guess getting this working on Gatsby is going to be quite a bit more work than I originally anticipated.
EDIT:
Turns out I just needed to move my code around taking into account how Gatsby works (get underlying data at build time and pass the data to a loadable component at runtime). Doh!
yeah figured that this has nothing to do with the library per-se because i am using it in production builds + ssr. the lib could address ssr though (or at least mention it in the readme)
Simple reproduction of the issue, by copy pasting the code in the readme https://codesandbox.io/s/fervent-brown-v0l20?file=/src/App.js
Simple reproduction of the issue, by copy pasting the code in the readme https://codesandbox.io/s/fervent-brown-v0l20?file=/src/App.js
clearly still the same issue. Initialising the lib on document.load
is just not sufficient for many use-cases.
Apologies for the delay. I'm willing to use componentDidMount. There is still this issue however. What if we called the "domDidLoad" function both in CDM and document.load?
but sometimes those properties aren't accurate/available yet at componentDidMount.
Could you provide an example for this case? Usually DOM related properties should be available on componentDidMount
otherwise React
would have a huge problem in the first place, no?
I will create an example repo some time today/tomorrow. For now, I will push a version with redundant code. If I can't reproduce it tomorrow, I will remove the redundancy.
This is released in v2.2.11 @jjrchrds @maerzhase @maxkrieger @goleary @olegrjumin
The codesandbox from @maxkrieger still doesn't work with v2.2.11
https://codesandbox.io/s/fervent-brown-v0l20?file=/src/App.js
@jsonkao you totally ignored what i wrote in #47. If you use progress
in gatsby and/or next.js you will end up with an error because this.state.offsetHeight
is undefined
. Why you didn't look at the PR? if you just wanted to introduce a quick-fix this was the solution for it. Your change is not a sufficient solution sorry.
The error you will see is:
TypeError: Failed to construct 'IntersectionObserver': The provided double value is non-finite.
because of undefined variables in rootMargin
option provided to the IntersectionObserver
Ah my bad, I hadn't noticed your changes to Step.js
. I'll merge that in now. Thanks for pointing it out again.
I'm trying to get the example working in Gatsby. The component renders, but onStepEnter is never actually called: