vue icon indicating copy to clipboard operation
vue copied to clipboard

fix(core): fix type check for Date prop

Open kikuchan opened this issue 2 years ago • 9 comments

It checks a prop by [object Date] instead of instanceof Date

This fixes a strange warning message: [Vue warn]: Invalid prop: type check failed for prop "date". Expected Date, got Date in some circumstances.

This happens when type: Date's Date is in another context, such as used in dev mode in nuxt for example.

I've written and attached a unit test script for nodejs to reproduce this issue easily, and I've confirmed by using the script that this PR resolves the issue. But I'm sorry I don't know how to write and include a unit test file suitable for this project, so please someone translate the test script suitable for the project.

What kind of change does this PR introduce? (check at least one)

  • [x] Bugfix
  • [ ] Feature
  • [ ] Code style update
  • [ ] Refactor
  • [ ] Build-related changes
  • [ ] Other, please describe:

Does this PR introduce a breaking change? (check one)

  • [ ] Yes
  • [x] No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • [x] It's submitted to the dev branch for v2.x (or to a previous version branch), not the master branch
  • [ ] When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
  • [ ] All tests are passing: https://github.com/vuejs/vue/blob/dev/.github/CONTRIBUTING.md#development-setup
  • [x] New/updated tests are included

If adding a new feature, the PR's description includes:

  • [ ] A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information: This also happens on Vue3 and should be fixed in the same way.

related issues in other projects: https://github.com/nuxt/nuxt.js/issues/5565 https://github.com/buefy/nuxt-buefy/issues/36

kikuchan avatar Sep 11 '21 18:09 kikuchan

Do you have a repro? don't attach files, post plain code only

posva avatar Sep 12 '21 10:09 posva

Oh, I'm sorry. This is a repro.

import Vue from 'vue';
import { createRenderer } from 'vue-server-renderer';
import vm from 'vm';

async function test(name, script, obj) {
  // console.log(`\n\n== ${name} =============================================`);

  let lastError = null;
  Vue.config.warnHandler = function (msg, vm, trace) {
    lastError = msg;
  }

  const result = vm.runInNewContext(script, obj)
  const html = await renderer.renderToString(result);

  // console.log(html);
  console.log('[test: ' + name + ']  => ' + (lastError ? `FAILED (${lastError})` : 'pass'));
}


const renderer = createRenderer()
const date = new Date();

const HelloWorld = {
  props: {
    date: Date,
  },
  template: "<div>Hello, It's {{ date }}</div>",
}

const script_1 = `
  HelloWorld = HelloWorld || {
    props: {
      date: Date,
    },
    template: "<div>Hello, It's {{ date }}</div>",
  }

  new Vue({
    components: { HelloWorld },
    template: '<HelloWorld :date="date" />',

    data: () => ({ date: date || new Date() }),
  })
`;


const script_2 = `
  HelloWorld = HelloWorld || {
    props: {
      date: Date,
    },
    template: "<div>Hello, It's {{ date }}</div>",
  }

  new Vue({
    components: { HelloWorld },
    template: '<HelloWorld :date="new Date()" />',
  })
`;

// pass if `HelloWorld` and `date` are in the same context
await test('A', script_1, { Vue, HelloWorld: null, date: null });
await test('B', script_1, { Vue, HelloWorld: null, date       }); // => FAILED
await test('C', script_1, { Vue, HelloWorld      , date: null }); // => FAILED
await test('D', script_1, { Vue, HelloWorld      , date       });

// It failed when `HelloWorld` is in the script context,
// so `new Date()` in the template is executed in this renderer context...?
await test('E', script_2, { Vue, HelloWorld: null }); // => FAILED
await test('F', script_2, { Vue, HelloWorld       });

kikuchan avatar Sep 12 '21 10:09 kikuchan

@posva I've made a much simpler code to reproduce, and now I know how to write a test code. Is this suitable?

kikuchan avatar Sep 12 '21 11:09 kikuchan

Do you have an actual boiled down reproduction? Like on a js fiddle ?

posva avatar Sep 12 '21 15:09 posva

It's based on https://github.com/buefy/nuxt-buefy/issues/36#issuecomment-486567317 but upgrade to the latest version of them. https://codesandbox.io/s/codesandbox-nuxt-forked-kdlsy

I think nuxt and nuxt-buefy setup is the easiest way to reproduce. I have no idea that why this combination triggers this issue, but I'm sure that multiple contexts are involved during the nuxt SSR process. For Nuxt, according to the https://github.com/nuxt/nuxt.js/issues/5565#issuecomment-724151739 , the behavior can be disabled by setting runInNewContext: false in the config file, but it's a workaround.

I've also checked value directly on https://github.com/vuejs/vue/blob/515467a618479792abedf01a7b1dcef2ac2a17ed/dist/vue.common.dev.js#L1746 by editing the file directly to inject console.log and/or to start REPL session in the context. After some investigations, the value is surely an instance of v8's internal Date but value instanceof Date returns false there, because a Date a constructor of value, and a Date there, are in a different context.

kikuchan avatar Sep 12 '21 16:09 kikuchan

That looks more like a big they should be fixing. Changing the global Date is not something tou should do

posva avatar Sep 12 '21 17:09 posva

Node has legitimate APIs to create a new running environment (context) for sandboxing or something else if you want. Changing the global Date variable by the user is usually a bad idea and should be prohibited strictly, but I think this is not a case because it is done automatically/internally/implicitly by Node/v8 itself.

Although the two Dates are valid native Date constructors for Node/v8, Vue type checking relies on the current global constructor to identify whether it is an instance of Date or not, thus it fails.

By the way, I've also come up with a reproduction code to prove there is a case that Dates have different contexts each other in normal browser environment.

<html>
  <head>
    <script src="https://cdn.jsdelivr.net/npm/[email protected]/dist/vue.js"></script>
  </head>
  <body>
    <div id="app">
      <div>{{ date }}</div>
      <test-component :date="date" />
    </div>

    <iframe src="child.html"></iframe>

    <script>
      const iframe = document.querySelector('iframe');
      iframe.onload = function () {
        const TestComponent = {
          template: '<div>{{ date }}</div>',
          props: {
            date: Date,
          }
        }

        new Vue({
          el: '#app',
          components: { TestComponent },

          data() {
            return {
              // XXX: Date instance comes from another context!
              date: iframe.contentWindow.date
            }
          },
        })
      };
    </script>
  </body>
</html>
<html>
  <head />
  <body>
    This is a child.
    <script>
      var date = new Date();
    </script>
  </body>
</html>

As you can see, I exploit <iframe> to make a specially crafted Date instance in this case.

Well... How about using the following code to fix the issue? value instanceof Date || Object.prototype.toString.call(value) === '[object Date]' In this way, I think it can minimize the chance of unexpected changes of behavior and can maximize backward compatibility.

kikuchan avatar Sep 13 '21 06:09 kikuchan

What's the status on this? I'm currently working on a Vue app with Nuxt and getting lots of the Expected Date, got Date errors. It's easy enough to ignore them but this is still a fix that would be useful (and make things way simpler for developers who are new to Vue + Nuxt).

AnnikaCodes avatar Feb 12 '22 02:02 AnnikaCodes

A temporary fix is to clone the date object before transmitting to the child component with buggyDate = new Date(buggyDate.getTime())

existe-deja avatar Mar 17 '22 10:03 existe-deja