Skip to content
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

Improve on document check #30

Closed
wants to merge 1 commit into from
Closed

Improve on document check #30

wants to merge 1 commit into from

Conversation

galvez
Copy link

@galvez galvez commented Aug 18, 2018

@yyx990803 Nuxt Edge is failing for me with this error:

✖ error ReferenceError: document is not defined
  at addStyle (node_modules/vue-style-loader/lib/addStylesClient.js:120:0)
  at addStylesToDom (node_modules/vue-style-loader/lib/addStylesClient.js:104:0)
  at addStylesClient (node_modules/vue-style-loader/lib/addStylesClient.js:58:0)

I traced it down to a SSR check that seems to be missing.

@galvez
Copy link
Author

galvez commented Aug 18, 2018

@clarkdo @atinux @yyx990803 for reference, here's the full log:

[3:41:02 PM] ReferenceError: document is not defined
at addStyle (node_modules/vue-style-loader/lib/addStylesClient.js:120:0)
at addStylesToDom (node_modules/vue-style-loader/lib/addStylesClient.js:104:0)
at addStylesClient (node_modules/vue-style-loader/lib/addStylesClient.js:58:0)
at Object.<anonymous> (node_modules/element-ui/lib/theme-chalk/display.css?4e3a:9:0)
at __webpack_require__ (webpack/bootstrap:19:0)
at Module.<anonymous> (server-bundle.js:18717:15)
at __webpack_require__ (webpack/bootstrap:19:0)
at server-bundle.js:85:18
at Object.<anonymous> (server-bundle.js:88:10)
at evaluateModule (/my-app/node_modules/vue-server-renderer/build.js:8349:21)
at /my-app/node_modules/vue-server-renderer/build.js:8407:18
at new Promise (<anonymous>)
at /my-app/node_modules/vue-server-renderer/build.js:8399:14
at Object.renderToString (/my-app/node_modules/vue-server-renderer/build.js:8575:9)
at Renderer.renderRoute (/my-app/node_modules/nuxt-edge/dist/nuxt.js:1889:41)
at Renderer.nuxtMiddleware (/my-app/node_modules/nuxt-edge/dist/nuxt.js:1447:31)

@galvez
Copy link
Author

galvez commented Aug 18, 2018

And __webpack_require__ fails with this file:

"webpack:///./node_modules/element-ui/lib/theme-chalk/display.css"

@galvez
Copy link
Author

galvez commented Aug 18, 2018

@yyx990803 We added a test to demonstrate:

nuxt/nuxt#3762

@yyx990803
Copy link
Member

SSR shouldn't ever hit this code path. There's something wrong else where.

@yyx990803 yyx990803 closed this Aug 18, 2018
@galvez
Copy link
Author

galvez commented Aug 18, 2018

@yyx990803 Interesting because the same project, with Nuxt 1.4.2 and VueStyleLoader 3.1.2 works! The scenario is loading ElementUI as a plugin in a SSR context. It works currently in my project with the aforementioned versions. When upgrading to Nuxt Edge (2.0) and VueStyleLoader 4.1.2, it starts giving me this error. I've looked long and hard through Nuxt's source code to try and pinpoint an error there, but it really does not seem to be doing anything wrong.

@galvez
Copy link
Author

galvez commented Aug 19, 2018

@yyx990803 @atinux more info -- this is the line that causes it to fail:

import 'element-ui/lib/theme-chalk/display.css'

from inside a Vue component in a Nuxt app. It works in 3.1.2 -- fails in 4.1.2. Now I'm puzzled if that's VueStyleLoader, Webpack or Nuxt that's getting something wrong.

@galvez
Copy link
Author

galvez commented Aug 19, 2018

@yyx990803 @atinux so my best guess, after losing a day researching this, is that nested imports (from inside the <script> tag) from external dependencies lose the proper webpack target. So perhaps a solution would be to adhere to a document-based check right from the beggining?

- module.exports.pitch = function (remainingRequest) {
-   var isServer = this.target === 'node'
+   var isServer = this.target === 'node' || typeof document === 'undefined'

@galvez
Copy link
Author

galvez commented Aug 20, 2018

@yyx990803 so you were right :) -- thanks @atinux for looking into this

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

Successfully merging this pull request may close these issues.

2 participants