-
Notifications
You must be signed in to change notification settings - Fork 46
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
package.json points to dist file that trips up Next.js #303
Comments
@andreasg123 I was actually recently trying this package in a Next.js setup with no issue, other than needing to add webpack: (config, { isServer }) => {
// Fixes npm packages that depend on `fs` module
if (!isServer) {
config.node = {
fs: 'empty'
}
}
return config
} to the config. @manzt I remember asking something similar, but I tried this recently with vizarr and didn't have this issue. Do you know what is happening here? |
Are you able to describe the issue in more detail? An error message would be useful. I'm not sure what is meant by "trips up Next.js," but you shouldn't need to transpile Viv (or any of your node_modules) to be able to use them in your build.
This is specific to deck.gl. The Webpack supports ESM natively. It is the most simple format for bundlers to resolve, which is why the default fields webpack tries to resolve are TL;DR - My guess is that the error you're experiencing has to do with next.js trying to resolve an import from a Node library like @ilan-gold pointed out ("fs"), and not identical fields in the package.json. My bet is that |
I also ran into the "fs" error with "geotiff" and fixed it the same way. I will attempt to determine which of the following may produce a different behavior for me when using Next.js:
Here is the error that I'm getting:
|
Thanks for updating with more detailed information! What version of node/npm are you using? Could you do me a favor and delete the |
Without the
I'm using Node 14.15.1 and Yarn 1.22.10. I'm using next 10.0.2 and React 16.13.1 in case that makes a difference. If you can suggest changes in |
Darn, thanks for trying it out! good to know. Let me look into the next configuration... If you don't mind, could you add back: |
Unfortunately, that didn't work, either:
In the meantime, I tried to add However, I had to change I wonder if my use of TypeScript is the main issue here (or that I need Babel for Jest). |
As other modules (e.g., GitHub) switch to ESM-only output, I think that it would be fine if you left things as they are. Maybe Next.js will handle this more gracefully in the future. Maybe you could note it in your README. |
@andreasg123 We recently changed out build system to Vite - did that help at all with this? Can we close this? |
Let me check tomorrow and get back to you. |
Unfortunately, I cannot include Viv 0.10.5 at all, without or with transpiling. I'll investigate this further and get back to you. Error with Viv 0.10.3 without transpiling:
Error with Viv 0.10.5 without transpiling:
Error with Viv 0.10.5 with transpiling:
|
I created a PR for Vite: vitejs/vite#4674 I manually modified |
Thanks! Hopefully that PR lands soon and we can upgrade. |
My confusion here is that viv is a purely client-side library; the module isn't intended to be run on the server. The part of the code with Have you looked into disabling SSR for Viv, or injecting Eg with |
In
package.json
, bothmain
andmodule
point to the same file containingimport
statements. Theimport
statements probably come from this line: https://github.com/hms-dbmi/viv/blob/master/rollup.config.js#L19Unfortunately, that trips up Next.js. It hides its Webpack configuration fairly well so that I'm not completely sure what it's doing exactly. I worked around it with next-transpile-modules but that plugin describes itself as "a big hack of the Next.js Webpack configuration".
My understanding is that
module
is supposed to point toesm
andmain
is supposed to point toes5
. At least, that's what deck.gl/core does (withesnext
as a third format): https://github.com/visgl/deck.gl/blob/master/modules/core/package.json#L19I'm not familiar with rollup but I could attempt to produce a PR if you don't get around to it first.
The text was updated successfully, but these errors were encountered: