Remove vue-server-renderer as a peer dependency

[eddyerburgh]

What problem does this feature solve?

renderToString was added in beta.12 , which uses vue-server-renderer.

I don't expect many users will call this method, as it's intended use is for testing server side code. Adding it as a peer dependency will warn users that they need to install it, even though they probably won't use the renderToString method.

What does the proposed API look like?

1. Remove vue-server-renderer as a peer dependency and handle it ourselves, using something like ensureRequire—https://github.com/vuejs/vue-jest/blob/master/lib/compilers/haml-compiler.js#L6. Users will still need to add additional config to webpack, because webpack will try to bundle native node modules in vue-server-renderer.
2. Split @vue/test-utils, and @vue/server-test-utils. @vue/test-utils will expose mount and shallow, @vue/server-test-utils will expose renderToString.

[lmiller1990]

Seems reasonable to me.

[timoschwarzer]

This breaks compatibility with tests that utilize webpack, too. For example with mocha-webpack:

[eddyerburgh]

@timoschwarzer A temporary workaround is to add this to your webpack config:

{
  node: {
    fs: 'empty',
    module: 'empty'
  }
}

See webpack-contrib/css-loader#447

This isn't a great solution, and we shouldn't force users to add extra config. My suggestion of checking the package ourselves won't solve this problem.

We could split vue-test-utils into two packages, vue-test-utils and vue-test-utils-server 🤔

[lmiller1990]

I thought about this a bit more. Is there any merit to removing/splitting?

[eddyerburgh]

It would mean people could bundle with webpack without having to add extra config, and it would solve the peer dependency issue

[lmiller1990]

Can't people bundle with webpack either way? Maybe I don't fully understand this part.

Considering both packages are maintained by the Vue core team, is it so bad to have a peer dependency? I understand it's better not to have them, though.

[eddyerburgh]

vue-server-renderer uses node modules, like path and fs.

To bundle these with webpack and run in node you need to add target:node to your config.

If you want to bundle to run tests in the browser (like with karma), you need to tell webpack not to bundle the native node modules vue-server-renderer uses (like path and fs).

This extra config makes it more difficult to set up webpack to compile tests for either node or the browser.

I don't think renderToString will be used by most users. It has only been requested as a feature recently. So we're making the setup for tests more difficult to support a few users.

Instead, we could turn vue-test-utils into a monorepo that generates two packages. @vue/test-utils, and @vue/server-test-utils. @vue/test-utils will expose mount and shallow, server-test-utils will expose renderToString.

That way, most users can carry on bundling their tests with webpack, without additional config. Any users who want to test SSR could download @vue/server-test-utils.

[lmiller1990]

Ok, I understand now. Thanks for that explanation.

In light of this, your proposal seems best.

[eddyerburgh]

I'm working on converting the repo into a monorepo this week. I'd appreciate feedback on the structure once I've finished 😄

EDIT: WIP—#447

[juhasev]

To fix both issues with webpack you need to set both fs and module empty in your webpack config:

node: {
    fs: "empty",
    module: "empty"
}

[iraklisg]

@eddyerburgh 👍

So we're making the setup for tests more difficult to support a few users.

Thanks for pointing this out 😛

[kdocki]

Even when you include vue-server-renderer and target: 'node' (or resolve node.fs and node.module to "empty") this workaround still shows CriticalDependencyErrors and is pretty annoying. The tests seem to work but you'll always be getting "red" tests because of the peer dependency is using fake target node environment. Please split these apart into packages that we can use for client side and server side like @eddyerburgh recommended.

EDIT:
I found that if I add these lines to my webpack config as recommended here then I don't seem to have issues anymore. I'm not sure what is happening here though.

{
    externals: [ require('webpack-node-externals')() ],
    devtool: 'inline-cheap-module-source-map'
}

[eddyerburgh]

@kdocki It's been split into seperate packages. We'll release soon

[gkatsanos]

I am getting these errors. Are they related?

WARNING in ./node_modules/jsdom/lib/jsdom/utils.js
203:21-40 Critical dependency: the request of a dependency is an expression
 @ ./node_modules/jsdom/lib/jsdom/utils.js
 @ ./node_modules/jsdom/lib/jsdom/living/node-filter.js
 @ ./node_modules/jsdom/lib/jsdom/living/index.js
 @ ./node_modules/jsdom/lib/jsdom/browser/Window.js
 @ ./node_modules/jsdom/lib/api.js
 @ ./node_modules/jsdom-global/index.js
 @ ./test/unit/setup.js

WARNING in ./node_modules/parse5/lib/index.js
55:23-49 Critical dependency: the request of a dependency is an expression
 @ ./node_modules/parse5/lib/index.js
 @ ./node_modules/jsdom/lib/jsdom/browser/htmltodom.js
 @ ./node_modules/jsdom/lib/jsdom/living/nodes/Document-impl.js
 @ ./node_modules/jsdom/lib/jsdom/living/generated/Document.js
 @ ./node_modules/jsdom/lib/jsdom/living/domparsing/DOMParser-impl.js
 @ ./node_modules/jsdom/lib/jsdom/living/generated/DOMParser.js
 @ ./node_modules/jsdom/lib/jsdom/living/index.js
 @ ./node_modules/jsdom/lib/jsdom/browser/Window.js
 @ ./node_modules/jsdom/lib/api.js
 @ ./node_modules/jsdom-global/index.js
 @ ./test/unit/setup.js

WARNING in ./node_modules/@vue/test-utils/dist/vue-test-utils.js
Module not found: Error: Can't resolve 'vue-server-renderer' in '/Users/katsanos/frontend/node_modules/@vue/test-utils/dist'
 @ ./node_modules/@vue/test-utils/dist/vue-test-utils.js 4706:15-45
 @ ./src/components/geofences/create/create.test.js
 @ ./src ^\.\/(?!main).+(\.js)$
 @ ./test/unit/index.js

ERROR in ./node_modules/sntp/lib/index.js
Module not found: Error: Can't resolve 'dns' in '/Users/katsanos/frontend/node_modules/sntp/lib'
 @ ./node_modules/sntp/lib/index.js 6:12-26
 @ ./node_modules/hawk/lib/index.js
 @ ./node_modules/request/request.js
 @ ./node_modules/request/index.js
 @ ./node_modules/request-promise-native/lib/rp.js
 @ ./node_modules/jsdom/lib/api.js
 @ ./node_modules/jsdom-global/index.js
 @ ./test/unit/setup.js

[eddyerburgh]

This has been removed in beta-13.

@vue/server-test-utils is now in its own package.

[gkatsanos]

@eddyerburgh do I need the server-test-utils if I dont do snapshot/dont use Jest?

[eddyerburgh]

No, server-test-utils is only useful for testing SSR code.

[cedrick-vargas]

Yeah! Finally, thanks @eddyerburgh and your team

[CraigHarley]

If like me you want renderToString to work on a client side rendered app (or you use another server than node). I couldn't get around the 'critical dependency error' or 'module is an expression' even after trying everything in this thread. Using mount was a pain because it didn't seem to recognise mocks on window or global.

This is how I got it to work:

Move all the SSR code into something like ssr-testing.js, import renderToString there, and ignore that file and your unit tests from webpack.
In webpack update your babel loaders regex to something like:
/^(?!(.*\.spec)|(ssr-testing)).*\.js$/

That way webpack will ignore the SSR stuff.

[terwer]

To fix both issues with webpack you need to set both fs and module empty in your webpack config:

node: {
    fs: "empty",
    module: "empty"
}

Thanks，it works