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

Node version should not use require('jsdom') to construct self variable #1483

Open
HriBB opened this issue Jun 12, 2018 · 16 comments
Open

Node version should not use require('jsdom') to construct self variable #1483

HriBB opened this issue Jun 12, 2018 · 16 comments

Comments

@HriBB
Copy link
Contributor

HriBB commented Jun 12, 2018

Description/Steps to reproduce

In src/node/self.js there is a check using require('jsdom'), to determine the type of self variable in node environments. This causes problems in some situations. For example when bundling paper.js with webpack to do server side rendering with react.

I am posting a stripped version of src/node/self.js for reference:

var path = require('path');
var parent = module.parent.parent,
    requireName = parent && path.basename(path.dirname(parent.filename));
requireName = /^paper/.test(requireName) ? requireName : 'paper';

var jsdom,
    self;

try {
    jsdom = require('jsdom');
} catch(e) {
    if (/\bjsdom\b/.test(requireName)) {
        throw new Error('Unable to load jsdom module.');
    }
}

if (jsdom) {
   var document = jsdom.jsdom('<html><body></body></html>', {
        url: 'file://' + process.cwd() + '/',
        features: {
            FetchExternalResources: ['img', 'script']
        }
    });
    self = document.defaultView;
    require('./canvas.js')(self, requireName);
    require('./xml.js')(self);
} else {
    self = {
        navigator: {
            userAgent: 'Node.js (' + process.platform + '; U; rv:' +
                    process.version + ')'
        }
    };
}

module.exports = self;

Link to reproduction test-case

N/A

Expected result

It's hard to tell what the right way to determine self variable is in this case. In my case, I don't need any of the jsdom stuff, and if I manually comment it out, everything works without any errors:

if (false && jsdom) {
    //...
} else {
    //...
}

I guess the expected result is that importing/requiring paper should not require any of the jsdom stuff, event when jsdom is present as a dependency of some other package.

Additional information

Paper.js 0.11.5
Node 9.2.0
Linux Ubuntu 16.04
Issues: #1347

@lehni
Copy link
Member

lehni commented Oct 5, 2018

We can't just remove that, see #739 for the reasons why it's there

@SBD580
Copy link

SBD580 commented Oct 12, 2018

I believe an option to set the requireName from some ENV varaiable should be good enough, so this could be controlled by build scripts

@lehni
Copy link
Member

lehni commented Oct 13, 2018

@SBD580 I don't think that would be enough. requireName only changes the error reporting behaviour, not the requiring itself.

@lehni
Copy link
Member

lehni commented Oct 13, 2018

@HriBB how are you bundling paper.js? Do you use the package from NPM? If so, why is your webpack ignoring these settings here:

paper.js/package.json

Lines 75 to 80 in 5245436

"canvas": false,
"jsdom": false,
"jsdom/lib/jsdom/living/generated/utils": false,
"source-map-support": false,
"./dist/node/self.js": false,
"./dist/node/extend.js": false

This should just work if you used the published versions through webpack

@SBD580
Copy link

SBD580 commented Oct 13, 2018

@lehni, maybe my issue is different. I don't have an error while bundling but rather at runtime saying module.parent is undefined (and thus can't access parent of undefined). Setting the requiredName fixed to paper-jsdom did the trick for me.

@lehni
Copy link
Member

lehni commented Oct 13, 2018

@SBD580 your issue is fixed here 6925888

We just need to roll out a new version soon.

@SBD580
Copy link

SBD580 commented Oct 13, 2018

Oh your right my bad (didn't find that on the search). I'm not sure the fix is correct though (commented on the commit)

@HriBB
Copy link
Contributor Author

HriBB commented Oct 14, 2018

@lehni I use package from npm "paper": "^0.11.5"

I am importing everything from paper/dist/paper-core

import paper from 'paper/dist/paper-core'
// or
import { Item, Point } from 'paper/dist/paper-core'

I also use react-paper-bindings which imports like this

BTW, to fix this problem in SSR, I override self variable in my server.js entry script

// paper.js ssr fix
global.self = {
  navigator: {
    userAgent: `Node.js (${process.platform}; U; rv:${process.version})`
  }
}

And this is my webpack config.server.js

const webpack = require('webpack')

const config = require('../src/config')

module.exports = {
  mode: process.env.NODE_ENV,
  target: 'node',
  devtool: 'source-map',
  entry: config.server.entry,
  output: config.server.output,
  resolve: {
    modules: [
      config.paths.src,
      config.paths.modules,
    ],
    extensions: ['.js', '.css', '.gql', '.graphql'],
  },
  module: {
    rules: [
      {
        test: /\.js$/,
        loader: 'babel-loader',
        exclude: /node_modules/,
        include: [config.paths.src],
      },
      {
        test: /\.(graphql|gql)$/,
        exclude: /node_modules/,
        loader: 'graphql-tag/loader',
      },
      {
        test: /\.node$/,
        use: 'node-loader'
      },
    ],
  },
  optimization: {
    minimize: false,
  },
  plugins: [
    new webpack.DefinePlugin({
      'process.env.NODE_ENV': JSON.stringify(process.env.NODE_ENV),
    }),
  ],
  node: {
    console: false,
    global: false,
    process: false,
    Buffer: false,
    __filename: false,
    __dirname: false,
    setImmediate: false,
  },
}

@lehni
Copy link
Member

lehni commented Oct 15, 2018

@HriBB intersting, thank you!

I think the solution is somewhere in here:

https://webpack.js.org/configuration/resolve/#resolve-mainfields

Currently we only target the browser target scenario with webpack. We may want to create another shim project that targets webpack on node for ssr, and disables these internal modules?

Could you create a simple test-case project that allows me to replicate your problem easily for testing? Ideally just a github repo. Thanks!

@HriBB
Copy link
Contributor Author

HriBB commented Oct 15, 2018

@lehni you're right. Haven't seen that config option yet.

I will create the reproduction repo, but I'm pretty busy ATM, so I cannot promise when.

@renschler
Copy link

renschler commented Apr 7, 2019

I think I'm running into the same issue as @HriBB

I have a very simple gatsby site (with SSR) that can reproduce the behavior. I only see the build error when I deploy on netlify, not when I run locally.

Screen Shot 2019-04-07 at 1 28 41 PM

Right now the repo is in bitbucket, i'll push it to github and link here when I get a chance.

Update: https://github.com/renschler/paperjs_example_repo

Still trying to figure out how to get it to fail locally, it only fails during the netlify build.

@blimpmason
Copy link

@renschler did you ever get this working? I'm getting the same build error when incorporating Paper into a Gatsby project, but only in local develop mode. Luckily it's not failing on Netlify's servers.

I've implemented the following in gatsby-node.js:

exports.onCreateWebpackConfig = ({ stage, loaders, actions }) => {
  if (stage === "build-html") {
    actions.setWebpackConfig({
      module: {
        rules: [{ test: /node_modules\/paper/, use: loaders.null() }],
      },
    });
  }
};

@ricardomatias
Copy link

I'm getting ModuleNotFoundError: Module not found: Error: Can't resolve 'jsdom' in '/home/ares/Projects/who-is-we/node_modules/paper/dist/node' with Next.js and Netlify. I'm using dynamic imports.

@amcc
Copy link

amcc commented Jun 1, 2021

same issue with a Gatsby project:

ERROR Generating SSR bundle failed

If you're trying to use a package make sure that 'jsdom/lib/jsdom/living/generated/utils' is installed. If you're trying to use a local file make sure that the path is correct.

Can't resolve 'jsdom/lib/jsdom/living/generated/utils' in '/usr/src/app/www/node_modules/paper/dist/node'

happen to be using https://github.com/psychobolt/react-paperjs, this imports Paper: import Paper from 'paper';

@markmanx
Copy link

The Unable to load jsdom module error will also occur if the package was compiled against a different version of Node than the one currently running it. Try deleting node_modules and reinstalling with the same version of Node.

The try/catch in src/node/self.js suppresses a lot of useful data about this particular error, I think it would be better to show the error detail in its entirety.

@noah-seltzer
Copy link

noah-seltzer commented Aug 12, 2021

I would also like to add that I ran into this error while attempting to load paper-jsdom in a worker thread (via the npm workerpool package).

I fixed this problem by:

  1. switching paper-jsdom with paper
  2. installing jsdom separately
  3. passing jsdom instance to paper at runtime
  const dom = new JSDOM(`<!DOCTYPE html>
  <html>
  <head>
  <!-- Load the Paper.js library -->
  <script type="text/javascript" src="js/paper.js"></script>
  </head>
  <body>
    <canvas id="myCanvas" resize></canvas>
  </body>
  </html>`)
  const canvas_element = dom.window.document.getElementById('myCanvas')
  paperscope.setup(canvas_element);

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

No branches or pull requests

9 participants