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

Use typeof instead of instanceof to check for functions (fix for [vuex] unknown local mutation type) #46

Merged
merged 1 commit into from
Feb 16, 2019

Conversation

SebastienTainon
Copy link
Contributor

@SebastienTainon SebastienTainon commented Feb 15, 2019

Hello,

After upgrading to Nuxt 2.4, I noticed vue-pathify had stopped working correctly when using state functions with server-side rendering, like this one:

export const state = () => ({
  user: null
});
export const mutations = make.mutations(state);
// It generates an empty array server-side and then "[vuex] unknown local mutation type"

I investigated and I discovered that the error was coming from the function check inside Vue-pathify: the "state instanceof Function" doesn't return true anymore while it is clearly a function.

Using instanceof to check for functions is not a good pratice because the "Function" type doesn't cover all the JS functions:

I haven't completely investigated why in the case of SSR and with the new versions of the libs that Nuxt 2.4 use, the functions that are exported by a component do not have the "Function" type anymore, anyway this should be fixed to avoid future problems.

Thanks!

@SebastienTainon SebastienTainon changed the title Use typeof instead of instanceof to check for functions Use typeof instead of instanceof to check for functions (fix for [vuex] unknown local mutation type) Feb 15, 2019
@davestewart
Copy link
Owner

Hey, thanks for checking into that.

So:

image

You're saying this is different on the server side?

Did you check in Pathify what is being passed into the check?

@SebastienTainon
Copy link
Contributor Author

SebastienTainon commented Feb 15, 2019

Hello @davestewart, thanks for responding so fast!

Not exactly: if I take your assertions, it produces exactly the same result. The problem is when the variable is exported, here is a reproducer:

export const state = () => ({
  user: null
});
export const mutations = make.mutations(state);

dist/vue-pathify.js:

function makeMutations (state) {
    console.log(typeof state === 'function'); // true
    console.log(state instanceof Function); // false
    console.log(state()); // {user: null}

    return getStateKeys(state)
      .reduce(function (obj, key) {
        var mutation = resolveName('mutations', key);
        obj[mutation] = function (state, value) {
          state[key] = value instanceof Payload
            ? value.update(state[key])
            : value;
        };
        return obj
      }, {})
  }

My setup is:

{
    "dependencies": {
        "@nuxtjs/apollo": "^4.0.0-rc4",
        "@nuxtjs/router": "^1.1.0",
        "apollo-cache-inmemory": "^1.4.2",
        "apollo-client": "^2.4.12",
        "apollo-link": "^1.2.6",
        "apollo-link-error": "^1.1.5",
        "apollo-link-http": "^1.5.9",
        "apollo-upload-client": "^10.0.0",
        "cross-env": "^5.2.0",
        "dotenv": "^5.0.1",
        "express": "^4.16.4",
        "graphql": "^14.1.1",
        "lodash": "^4.17.10",
        "node-fetch": "^2.3.0",
        "nuxt": "^2.4.3",
        "nuxt-buefy": "^0.3.3",
        "vee-validate": "^2.1.7",
        "vue-apollo": "^3.0.0-beta.28",
        "vue-i18n": "^8.8.1",
        "vuex-pathify": "^1.1.3"
    },
    "devDependencies": {
        "graphql-tag": "^2.10.1",
        "node-sass": "^4.11.0",
        "nodemon": "^1.18.10",
        "onchange": "^5.2.0",
        "sass-loader": "^7.1.0"
    }
}

@davestewart davestewart merged commit 6cbb37a into davestewart:develop Feb 16, 2019
@davestewart
Copy link
Owner

davestewart commented Feb 16, 2019

thanks for responding so fast!

No problem - I've been terrible recently!

OK, seems strange, but the fix will of course be compatible.

I'll investigate next week, and will do a release as well.

Thanks!

@SebastienTainon
Copy link
Contributor Author

Thanks! I admit that I haven't found the exact cause of the issue; my guess would be a change in the Babel plugins used by Nuxt or one of its dependencies that converts exported functions into another type, but I haven't found any mention of that.

@davestewart
Copy link
Owner

There are only 7 JS types; function being the only function type, so not sure what is going on there.

Let's see what happens this week :)

@SebastienTainon
Copy link
Contributor Author

Yes sure about the primary types, which start with a lowercase letter and are the possible outputs of typeof, I was speaking about the object types for which "instanceof" is doing the check, with an uppercase letter; there could be an object-type that "inherits" somehow from Function for example.

@SebastienTainon
Copy link
Contributor Author

SebastienTainon commented Feb 26, 2019

I've taken the time to investigate a bit this evening. If I do:

import {make} from "vuex-pathify";

Then it doesn't work server-side: the function is not instanceof Function.

However, if I do instead:

import {make} from "~/../node_modules/vuex-pathify/dist/vuex-pathify.esm";

It works both server-side and client-side! So my guess is that there is something in the "universalization" header of the vuex-pathify/dist/vuex-pathify.js file that changes the prototype of objects server-side, when the module is not an ES module.

@davestewart
Copy link
Owner

davestewart commented Feb 26, 2019

Hmm. That's interesting. I don't know enough about the internals of the bundling to know what that could be, but if you think it makes a difference, thanks for catching and investigating.

Did this happen only on the server side?

Can you make webpack aliases on the server side, so you could do:

resolve: {
  aliases: {
    vuex-pathify: './node_modules/vuex-pathify/dist/vuex-pathify.esm'
  }
}

Then just import as normal.

?

@SebastienTainon
Copy link
Contributor Author

Thanks for the input! I cannot make it work naming the alias exactly the same as the module name, there seems to be a precedence order that make Webpack check the node module first. If I name the alias differently (and import consequently), everything works well.

To answer your first question, I haven't had any trouble with vuex-pathify client-side, the problem was server-side. Using the ESM module server-side fixes it without messing the client-side either.

I've read that Webpack 4 is configured to import ES modules first (see here: https://webpack.js.org/configuration/resolve/#resolvemainfields) but I don't understand why it doesn't actually happen with these Nuxt version and libs...

@davestewart
Copy link
Owner

I had to reconfigure a project today using Post CSS and after half an hour of reading through docs and having about 10 tabs open of different articles saying different things, my head started to hurt.

Setup sucks!

@davestewart
Copy link
Owner

@SebastienTainon
Copy link
Contributor Author

Hello, thank you I've just downloaded the 1.2.0 from npm, I indeed have the new folder "types" in it but in the dist/vuex-pathify.(esm.)js files I still have the "instanceof Function" checks, it seems that my PR was not released in the 1.2.0...

@davestewart
Copy link
Owner

Oh shiiiiiiiit. I completely forgot to build! It's been that long since I updated that I forgot that step. Thanks for pointing that out. Stand-by for a 1.2.1 update

@davestewart
Copy link
Owner

OK, all done. Thanks!

@SebastienTainon
Copy link
Contributor Author

SebastienTainon commented Mar 12, 2019

Ah ah, no problem, thank you, everything is working great now! :D

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

Successfully merging this pull request may close these issues.

2 participants