Skip to content
This repository has been archived by the owner on Jan 14, 2025. It is now read-only.

ReferenceError: Element is not defined on SSR #561

Closed
lucasecdb opened this issue Dec 24, 2018 · 20 comments
Closed

ReferenceError: Element is not defined on SSR #561

lucasecdb opened this issue Dec 24, 2018 · 20 comments

Comments

@lucasecdb
Copy link
Contributor

Hello, I'm getting a ReferenceError: Element is not defined when I try to SSR the Drawer component, running on version 0.7.1.

The issue is probably some optimization on the webpack build process where it removes "dead-code", because the error occurs on the tabbable npm package, imported from focus-trap, specifically on this line. But, as you can see, it has a check specifically to avoid this issue when running on node environments. I have checked the minified code and have confirmed that the check for the typeof Element is optimized away.

Unfortunately I don't have a reproducible codesandbox snippet as it doesn't execute the code with SSR 😔

@moog16
Copy link

moog16 commented Dec 26, 2018

There was an issue a while back where someone couldn't render the list correctly because of minification (#492).

What does the typeof Element become in the minified code? Sadly this isn't our code and we don't have much control over it. Perhaps what I could do is not run the uglifier over the focus-trap package.

@lucasecdb
Copy link
Contributor Author

In the minified code it doesn't have the typeof Element ternary, just the part after the colon.

Perhaps what I could do is not run the uglifier over the focus-trap package.

Yeah, this should work. Also, it would be nice to have a test case covering this issue, so it doesn't happen again accidentally.

@lucasecdb
Copy link
Contributor Author

lucasecdb commented Dec 26, 2018

function _(module,exports) {
  var candidateSelectors = ['input','select','textarea','a[href]','button','[tabindex]','audio[controls]','video[controls]','[contenteditable]:not([contenteditable="false"])'];
  var candidateSelector = candidateSelectors.join(',');
  var matches = Element.prototype.matches || Element.prototype.msMatchesSelector || Element.prototype.webkitMatchesSelector;
   
  function tabbable(el,options) {
    // ...
  }
  // ...
}

This is how the tabbable package is on the index.js inside the dist folder of the npm module. What I find weird is that on the 0.6.2 version the ternary is still there, just as on the actual package.

@moog16
Copy link

moog16 commented Dec 26, 2018

So this effectively disappears? typeof Element === 'undefined' ? function () {}

@lucasecdb
Copy link
Contributor Author

Yeah, exactly.

@moog16
Copy link

moog16 commented Dec 26, 2018

Also I think we switched from react-focus-trap to focus-trap during v0.7.0. So that could be why its still in v0.6.2.

@moog16
Copy link

moog16 commented Dec 26, 2018

I have an idea: add this package to the externals during the npm run build step. This should force the drawer package to look at node_modules instead of trying to minify it. I think this should work

@lucasecdb
Copy link
Contributor Author

adding it as an external mean it'll be a peer dependency?

@moog16
Copy link

moog16 commented Dec 26, 2018

hoflish added a commit to hoflish/sbatashop that referenced this issue Dec 27, 2018
* Add material design icons
* Add categories nav
* Fix Drawer SSR

Issues:
  - material-components/material-components-web-react#561
@4cm4k1
Copy link
Contributor

4cm4k1 commented Dec 31, 2018

Hi, I just saw this and realized it's related to the poorly worded issues I filed. Maybe it would help to reference them here: #462 material-components/material-components-web#4104

@moog16
Copy link

moog16 commented Jan 2, 2019

Thanks @4cm4k1! Seems like it is a MDC Web issue not react issue. I still haven't had the chance to look into this one. I will deprioritize this on my end since its Web related.

@alexanderjeurissen
Copy link

alexanderjeurissen commented Jan 3, 2019

@moog16 @4cm4k1 issue material-components/material-components-web#4104 has been resolved, the fix (upgrading focus-trap dependency for the drawer and dialog components have been merged. We need to await a new release and bump the mdc-web dependency accordingly to resolve this for materila-components-web-react.

@moog16
Copy link

moog16 commented Jan 3, 2019

@alexanderjeurissen MDC Web will release their next cut this Monday. We will be skipping v0.42.0 and going straight to v0.43.0.

@moog16
Copy link

moog16 commented Jan 4, 2019

This hasn't been slated for 0.9.0 but seems like a blocker for some people. Can someone update to 0.43.0 when it is landed next week?

The main contributors of this project are moving to MDC Web's typescript conversion project (which will last for about 1 - 3 months). All issues/prs will be reviewed, however we will not be able to contribute. I will add more detail in our Roadmap, but just wanted to see if there are any volunteers who could fix this.

@4cm4k1
Copy link
Contributor

4cm4k1 commented Jan 5, 2019

@moog16 Is it just updating the package.json to use 0.43.0 of MDC Web? Or I guess I'll see what you add to the roadmap. Since most of these SSR things are kinda blockers for me, I'd be happy to volunteer to help with this.

@moog16
Copy link

moog16 commented Jan 5, 2019

awesome! Yes its updating to 0.43.0 (come 1/7/19). I don't think drawer has any actual changes.

@moog16
Copy link

moog16 commented Jan 7, 2019

0.43.0 released today!

@lucasecdb
Copy link
Contributor Author

lucasecdb commented Mar 1, 2019

So, I've figured that with the code from #706, you can add the following to your package.json to fix the SSR problems I was having:

{
  "resolutions": {
    "@material/ripple": "1.0.0-0",
    "@material/tab-scroller": "1.0.0-0",
    "@material/drawer": "1.0.0-0"
  }
}

If you do not want to wait for the code to be released, you can link the built package from the rc0.11.0 branch and link it in your repo. I'm closing this since the fix was already merged.

edit: This, of course, will only work for local development, if you use a cloud-based platform for your development or have tests running in a CI, they will break.

@4cm4k1
Copy link
Contributor

4cm4k1 commented Mar 1, 2019

Wow, awesome! Thanks, @lucasecdb! Also, thanks @moog16; I'm sorry I didn't have time to continue working on my PR related to this. I started a new software engineering job, so that has been taking a lot of my time lately, though I think as I settle in there, I will be able to spend more of my time on GitHub again. 💜

@moog16
Copy link

moog16 commented Mar 2, 2019

@lucasecdb thanks for finding a fix for this. I finally have some more bandwidth now that we will be releasing 1.0.0 next week. Going to try upgrading all the packages. :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants