nuxt / http

Universal HTTP Module for Nuxt.js
https://http.nuxtjs.org
MIT License
229 stars 51 forks source link

fix(ky-universal): server importing es bundle #121

Closed privatenumber closed 4 years ago

privatenumber commented 4 years ago

Using @nuxt/http on Vercel via @nuxtjs/now-builder and was getting the following error on the server-side:

Screen Shot 2020-08-20 at 1 13 45 AM

I added a console.log(fetch); (as you can see above) and it was importing the ESM version (node-fetch/lib/index.es.js). I specified the specific CJS build explicitly.

codecov-commenter commented 4 years ago

Codecov Report

Merging #121 into dev will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev     #121   +/-   ##
=======================================
  Coverage   97.18%   97.18%           
=======================================
  Files           1        1           
  Lines          71       71           
  Branches       39       39           
=======================================
  Hits           69       69           
  Misses          2        2           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4ef88e4...cccff9d. Read the comment docs.

pi0 commented 4 years ago

Hi @privatenumber and thanks for pull-request :+1:

There is a strategy called interopDefault which we can do something like this:

const _interopDefault = (ex) => ex && (typeof ex === 'object') && 'default' in ex) ? ex['default'] : ex

const fetch = _interopDefault(require('node-fetch'))
privatenumber commented 4 years ago

Updated

privatenumber commented 4 years ago

Actually, there's another bug. It expects the CJS version

Screen Shot 2020-08-20 at 1 50 59 PM

We'd have to either add a complicated named exports interop, or explicitly import the CJS version like it did before.


Sorry, noticed you approved it so had to comment quick before the merge

pi0 commented 4 years ago

Actually, there's another bug. It expects the CJS version

@privatenumber Would you please elaborate more? What's the error with latest change?

privatenumber commented 4 years ago

@pi0 Updated my comment

pi0 commented 4 years ago

https://github.com/nuxt/http/releases/tag/v0.5.11 released :D

privatenumber commented 4 years ago

Thank you @pi0 🔥