Skip to content
This repository has been archived by the owner on Dec 30, 2022. It is now read-only.

SSR is broken #1054

Closed
ArcoMul opened this issue Sep 7, 2021 · 3 comments · Fixed by #1104
Closed

SSR is broken #1054

ArcoMul opened this issue Sep 7, 2021 · 3 comments · Fixed by #1104

Comments

@ArcoMul
Copy link

ArcoMul commented Sep 7, 2021

Bug 🐞

What is the current behavior?

The SSR example causes an error, which I'm also receiving in my own implementation.

TypeError: (intermediate value)(intermediate value)(intermediate value) is not a constructor
    at a (node_modules/vue-instantsearch/vue2/es/src/util/createServerRootMixin.js:1:363)
    at app.js:14981:1049
    at async Promise.all (index 0)

Make a sandbox with the current behavior

https://github.com/algolia/vue-instantsearch/tree/master/examples/ssr

The only things I changed, are:

   "dependencies": {
     "algoliasearch": "^4.0.1",
     "instantsearch.css": "^7.1.1",
-    "vue": "^2.6.7",
-    "vue-instantsearch": "file:../../",
+    "vue": "2.6.11",
+    "vue-instantsearch": "^4.0.0",
     "vue-router": "^3.0.2",
-    "vue-server-renderer": "^2.6.11"
+    "vue-server-renderer": "2.6.11"
   },
   "devDependencies": {
     "@akryum/vue-cli-plugin-ssr": "^0.5.0",
@@ -25,7 +25,7 @@
     "babel-eslint": "^10.0.1",
     "eslint": "^5.14.1",
     "eslint-plugin-vue": "^5.2.2",
-    "vue-template-compiler": "^2.6.7"
+    "vue-template-compiler": "2.6.11"
   },

What is the expected behavior?

Not receiving that error

Does this happen only in specific situations?

No

What is the proposed solution?

Seems like the code which errors is trying to use the this passed in findResultsState as a constructor. I don't understand the context, so that's as far as I got :-)

    serverPrefetch() {
      return this.instantsearch.findResultsState({
        component: this, // I think the value passed here is being used as a constructor, which it isn't
        renderToString
      });
    },

What is the version you are using?

4.0.0

@ArcoMul ArcoMul changed the title SSR example gives an error SSR is broken Sep 7, 2021
@Haroenv
Copy link
Contributor

Haroenv commented Sep 7, 2021

This should be the fixed version: https://codesandbox.io/s/crimson-bush-ni2ii?file=/src/main.js. Not sure at which point Vue made the component name required, but before component(options) worked, now it seems like it needs to be component(name, options).

function renderToString(app) {
  import('vue-server-renderer/basic').then(({ default: _renderToString }) => {
    return new Promise((resolve, reject) => {
      _renderToString(app, (err, res) => {
        if (err) reject(err);
        resolve(res);
      });
    });
  });
}

function cloneComponent(componentInstance, { mixins = [] } = {}) {
  const options = {
    serverPrefetch: undefined,
    fetch: undefined,
    _base: undefined,
    name: 'ais-ssr-root-component',
  };

  let app;

  // copy over global Vue APIs
  options.router = componentInstance.$router;
  options.store = componentInstance.$store;

  const Extended = componentInstance.$vnode
    ? componentInstance.$vnode.componentOptions.Ctor.extend(options)
    : // this different, in the source code is only `Vue.component(Object.assign({}, componentInstance.$options, options))`
      Vue.component(
        options.name,
        Object.assign({}, componentInstance.$options, options)
      );

  app = new Extended({
    propsData: componentInstance.$options.propsData,
    mixins: [...mixins],
  });

  // https://stackoverflow.com/a/48195006/3185307
  app.$slots = componentInstance.$slots;
  app.$root = componentInstance.$root;
  app.$options.serverPrefetch = [];

  return app;
}

// ...
    mixins: [
      createServerRootMixin({
        $cloneComponent: cloneComponent,
        indexName: 'instant_search',
        searchClient,

// ...
    serverPrefetch() {
      return this.instantsearch.findResultsState({
        component: this,
        renderToString,
      });
    },

@Haroenv
Copy link
Contributor

Haroenv commented Sep 7, 2021

If you want an inbetween solution while we figure out what went wrong, you can also use Vue InstantSearch v3

Haroenv added a commit that referenced this issue Jan 28, 2022
fixes #1054

Essentially the problem is that $vnode is usually available, but not when the this is a root Vue instance. In that case we are in the "Vue.component" case, which before now always was wrong (it takes two arguments, not one)
Haroenv added a commit that referenced this issue Jan 31, 2022
Essentially the problem is that $vnode is usually available, but not when the this is a root Vue instance. In that case we are in the "Vue.component" case, which before now always was wrong (it takes two arguments, not one)

Same as #1104
see also #1054
Haroenv added a commit that referenced this issue Jan 31, 2022
Essentially the problem is that $vnode is usually available, but not when the this is a root Vue instance. In that case we are in the "Vue.component" case, which before now always was wrong (it takes two arguments, not one)

Same as #1104
see also #1054
Haroenv added a commit that referenced this issue Jan 31, 2022
* fix(ssr): extend component correctly if at root (vue2)

fixes #1054

Essentially the problem is that $vnode is usually available, but not when the this is a root Vue instance. In that case we are in the "Vue.component" case, which before now always was wrong (it takes two arguments, not one)

* not only

* Update src/util/__tests__/createServerRootMixin.test.js

Co-authored-by: Sarah Dayan <5370675+sarahdayan@users.noreply.github.com>

Co-authored-by: Sarah Dayan <5370675+sarahdayan@users.noreply.github.com>
Haroenv added a commit that referenced this issue Jan 31, 2022
* fix(server): extend component correctly if at root

Essentially the problem is that $vnode is usually available, but not when the this is a root Vue instance. In that case we are in the "Vue.component" case, which before now always was wrong (it takes two arguments, not one)

Same as #1104
see also #1054

* Update src/util/createServerRootMixin.js

* make the funky new test pass

* update test name, as in #1104

* Update src/util/__tests__/createServerRootMixin.test.js

Co-authored-by: Sarah Dayan <5370675+sarahdayan@users.noreply.github.com>

Co-authored-by: Sarah Dayan <5370675+sarahdayan@users.noreply.github.com>
@Haroenv
Copy link
Contributor

Haroenv commented Jan 31, 2022

We have just finished a comprehensive refresh of the server-side interaction with routing in the case of Vue InstantSearch. This managed to close this issue for both v3 in 3.9.0 and v4 in 4.3.2.

The pull requests in question are:

If the component at the root is the one doing findResultsState, it doesn't have access to $vnode, and in that branch the component name is required to be passed via the first argument.

This is a little more involved, but fixes the issue where routing is not taken in account. Essentially in InstantSearch it was changed so that routing only gets read on .start, but Vue InstantSearch was faking start.

Luckily, in a recent version of InstantSearch, some functionality of server rendering / initial results has moved inside InstantSearch itself, and it avoids the issue, as we can use regular 'start' from then on.

Haroenv added a commit to algolia/instantsearch that referenced this issue Dec 28, 2022
…nstantsearch#1104)

* fix(ssr): extend component correctly if at root (vue2)

fixes algolia/vue-instantsearch#1054

Essentially the problem is that $vnode is usually available, but not when the this is a root Vue instance. In that case we are in the "Vue.component" case, which before now always was wrong (it takes two arguments, not one)

* not only

* Update src/util/__tests__/createServerRootMixin.test.js

Co-authored-by: Sarah Dayan <5370675+sarahdayan@users.noreply.github.com>

Co-authored-by: Sarah Dayan <5370675+sarahdayan@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants