-
-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added guard against HTMLElement in callSubscriber #62
Conversation
@netsgnut this syntax only works when you use TypeScript. Maybe that's the issue? 🤔 |
Thank you for the swift response, @ZeeCoder ! While the code above is certainly TypeScript, I guess it is not really the reason here. 🤔 I have set up a super minimal repo demonstrating this potential issue. I have used |
@ZeeCoder I can confirm this throws for us too. Unfortunately have to rewrite this only my own instead of including this package to solve it. |
I get the issue now, seems like Next.js is using js-dom for SSR, which does not include the HTMLElement global. I'll need to refactor that test to replicate the issue before a fix is applied. Thanks for flagging. 🙏 |
@ZeeCoder @netsgnut Here is a repro, you need to use ForwardRef to get into this issue: const React = require("react");
const ReactDOMServer = require("react-dom/server");
const useResizeObserver = require("use-resize-observer");
const ForwardRef = React.forwardRef(function (props, ref) {
return React.createElement(
"div",
{ style: { width: 100, height: 200 } },
`${width}x${height}`
);
});
function Test() {
const ref = React.useRef(null);
useResizeObserver({
ref: ref,
});
return React.createElement(ForwardRef, { ref });
}
ReactDOMServer.renderToString(React.createElement(Test));
|
I can confirm the changes from this pull request fix the issue for me (also using Next.js SSR) |
This is now fixed in 7.0.1! 🎉 The related commit in case anyone is interested in the final solution: 599cace Thanks for everyone involved in this issue, your contributions helped and somewhat guided the final solution. 🙏 |
Thank you! |
Hi!
Thank you for the library! We are using v7.0.0 in one of the Next.js projects recently.
In one of the few instances that we are using this library, we are providing a
However when we try to build it with
next build
, there will be aReferenceError: HTMLElement is not defined
....which I believe is caused by the
HTMLElement
being referenced in the server-side, when it is trying to resolve the element.Admittedly I am not sure if we are actually abusing/misusing the library, or if the patch is merely hiding some bugs/issues lying deeper within the logic, since in another component that uses the
ref
from the library (just like what README says):...does not result in such error.
What do you think?