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

SSR: textarea domProps keeps falsy values #10803

Closed
KaelWD opened this issue Nov 6, 2019 · 4 comments · Fixed by #11121
Closed

SSR: textarea domProps keeps falsy values #10803

KaelWD opened this issue Nov 6, 2019 · 4 comments · Fixed by #11121

Comments

@KaelWD
Copy link
Contributor

KaelWD commented Nov 6, 2019

Version

2.6.10

Reproduction link

https://codesandbox.io/s/codesandbox-nuxt-vdcv8

Steps to reproduce

View page source

What is expected?

<textarea id="input-62" rows="5"></textarea>

What is actually happening?

<textarea id="input-62" rows="5">null</textarea>

Similar to #9231

Repro in vue/test/ssr/ssr-string.spec.js:

  it('falsy domProps value', done => {
    renderVmWithOptions({
      render (h) {
        return h('div', [
          h('textarea', {
            domProps: {
              value: null
            }
          })
        ])
      }
    }, result => {
      expect(result).toContain(
        '<div data-server-rendered="true"><textarea></textarea></div>'
      )
      done()
    })
  })

Relevant vuetify code:
https://github.com/vuetifyjs/vuetify/blob/243a7c34a1c58dff3753ad35dded13ba5002c8eb/packages/vuetify/src/components/VTextarea/VTextarea.ts#L86-L92
https://github.com/vuetifyjs/vuetify/blob/243a7c34a1c58dff3753ad35dded13ba5002c8eb/packages/vuetify/src/components/VTextField/VTextField.ts#L357-L361

@posva
Copy link
Member

posva commented Nov 7, 2019

Are you sure this comes from vue or that it hasn't been fixed already but not released? I added your test to the test suite and it passes

@KaelWD
Copy link
Contributor Author

KaelWD commented Nov 7, 2019

Yep

Could it be a node version thing (seems unlikely)? I'm on 12.5.0

~/Documents/vuejs/vue dev*
❯ git rev-parse HEAD 

4821149b8bbd4650b1d9c9c3cfbb539ac1e24589

~/Documents/vuejs/vue dev*
❯ git --no-pager diff
diff --git a/test/ssr/ssr-string.spec.js b/test/ssr/ssr-string.spec.js
index e18ca2ae..7dbb1ffb 100644
--- a/test/ssr/ssr-string.spec.js
+++ b/test/ssr/ssr-string.spec.js
@@ -4,6 +4,25 @@ import { createRenderer } from '../../packages/vue-server-renderer'
 const { renderToString } = createRenderer()
 
 describe('SSR: renderToString', () => {
+  it('falsy domProps value', done => {
+    renderVmWithOptions({
+      render (h) {
+        return h('div', [
+          h('textarea', {
+            domProps: {
+              value: null
+            }
+          })
+        ])
+      }
+    }, result => {
+      expect(result).toContain(
+        '<div data-server-rendered="true"><textarea></textarea></div>'
+      )
+      done()
+    })
+  })
+
   it('static attributes', done => {
     renderVmWithOptions({
       template: '<div id="foo" bar="123"></div>'

~/Documents/vuejs/vue dev*
❯ yarn test:ssr
yarn run v1.19.1
$ npm run build:ssr && jasmine JASMINE_CONFIG_PATH=test/ssr/jasmine.js
npm WARN lifecycle The node binary used for scripts is /tmp/yarn--1573128640608-0.24686075565520493/node but npm is using /home/kael/.nvm/versions/node/v12.5.0/bin/node itself. Use the `--scripts-prepend-node-path` option to include the path for the node binary npm was executed with.

> [email protected] build:ssr /home/kael/Documents/vuejs/vue
> npm run build -- web-runtime-cjs,web-server-renderer

npm WARN lifecycle The node binary used for scripts is /tmp/yarn--1573128640608-0.24686075565520493/node but npm is using /home/kael/.nvm/versions/node/v12.5.0/bin/node itself. Use the `--scripts-prepend-node-path` option to include the path for the node binary npm was executed with.

> [email protected] build /home/kael/Documents/vuejs/vue
> node scripts/build.js "web-runtime-cjs,web-server-renderer"

dist/vue.runtime.common.dev.js 217.93kb
dist/vue.runtime.common.prod.js 63.19kb (gzipped: 22.83kb)
packages/vue-server-renderer/build.dev.js 247.54kb
packages/vue-server-renderer/build.prod.js 80.21kb (gzipped: 29.28kb)
packages/vue-server-renderer/basic.js 334.34kb
packages/vue-server-renderer/server-plugin.js 2.91kb
packages/vue-server-renderer/client-plugin.js 3.03kb
Browserslist: caniuse-lite is outdated. Please run next command `yarn upgrade caniuse-lite browserslist`
Started
........................................F................................................................................................................

Failures:
1) SSR: renderToString falsy domProps value
  Message:
    Expected '<div data-server-rendered="true"><textarea>null</textarea></div>' to contain '<div data-server-rendered="true"><textarea></textarea></div>'.
  Stack:
    Error: Expected '<div data-server-rendered="true"><textarea>null</textarea></div>' to contain '<div data-server-rendered="true"><textarea></textarea></div>'.
        at toContain (/home/kael/Documents/vuejs/vue/test/ssr/ssr-string.spec.js:19:22)
        at cb (/home/kael/Documents/vuejs/vue/test/ssr/ssr-string.spec.js:1621:5)
        at RenderContext.cb [as done] (/home/kael/Documents/vuejs/vue/packages/vue-server-renderer/build.dev.js:9205:13)
        at RenderContext.done (/home/kael/Documents/vuejs/vue/packages/vue-server-renderer/build.dev.js:2588:19)

153 specs, 1 failure
Finished in 8.334 seconds

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

@posva
Copy link
Member

posva commented Nov 8, 2019

I was running the unit test suite, I forgot we have different script for that 🤦‍♂️

@danielsuguimoto
Copy link
Contributor

danielsuguimoto commented Feb 18, 2020

@posva Hi! I sent a pull request for this issue. Could you review it for me? Please, feel free to assign it to anybody else if you want to. Thanks!

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

Successfully merging a pull request may close this issue.

3 participants