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

ssrBuild with async Vue3 components / routes , trouble with import to require conversion in built files. #764

Closed
tbgse opened this issue Aug 29, 2020 · 19 comments · Fixed by #951
Labels
bug: upstream Bug in a dependency of Vite has workaround

Comments

@tbgse
Copy link

tbgse commented Aug 29, 2020

Describe the bug

I've been spending some time with the mostly undocumented ssrBuild feature of Vite + Vue3, so this might not actually be a bug but working as intended and i might be doing something wrong.

I am trying to run the ssrBuild with a router that defines some async components as below:

export default function (type) {
  const routerHistory = type === 'client' ? createWebHistory() : createMemoryHistory();
  
  return createRouter({
    history: routerHistory,
    routes: [
      { path: '/', component: Home, props: true },
      { path: '/a', component: () => import('./components/PageA.vue'), props: true },
      { path: '/b', component: () => import('./components/PageB.vue'), props: true },
    ]
  });
}

However, once i built my entry files with ssrBuild it converts the router part in the entry file to:

function createRouter$1(type) {
  const routerHistory = type === "client" ? createWebHistory() : createMemoryHistory();
  return createRouter({
    history: routerHistory,
    routes: [
      {path: "/", component: script$2, props: true},
      {path: "/a", component: () => Promise.resolve().then(function() {
        return require("./PageA.161ddab2.js");
      }).default, props: true},
      {path: "/b", component: () => Promise.resolve().then(function() {
        return require("./PageB.f65a13cd.js");
      }).default, props: true}
    ]
  });
}

The problem here is how the requires are defined:

 return require("./PageA.161ddab2.js");

The above code does end up not being able to find the component. If i manually go into the built files and add a .default behind this, the code works fine and my pages load / hydrate correctly.

 return require("./PageA.161ddab2.js").default;

since the exported pages all have default exports, this seems to be logical. E.g. in PageA:

"use strict";
var renderer = require("@vue/server-renderer");
var script = {
  name: "PageA",
  props: {
    msg: String
  },
  data() {
    return {
      randomNumber: 0,
      initialized: false,
      count: 0
    };
  }
};
function ssrRender(_ctx, _push, _parent, _attrs, $props, $setup, $data, $options) {
  _push(`<h1${renderer.ssrRenderAttrs(_attrs)}>I AM PAGE A</h1>`);
}
script.ssrRender = ssrRender;
script.__file = "src/components/PageA.vue";
exports.default = script;

Is this actually a bug with vite where it should be adding the .default to the required components? Am I doing something wrong here and should i define my imports differently? Happy to provide additional info if needed.

System Info

  • required vite version: ^1.0.0-rc.4
  • required Operating System: win10 with WSL1
  • required Node version: v14.7.0
@antfu
Copy link
Member

antfu commented Aug 31, 2020

Related to #720

@underfin
Copy link
Member

underfin commented Sep 2, 2020

For async component, you should use defineAsyncComponent wrapper it.The rfc is here https://github.com/vuejs/rfcs/blob/async-component/active-rfcs/0026-async-component-api.md

@underfin underfin closed this as completed Sep 2, 2020
@tbgse
Copy link
Author

tbgse commented Sep 2, 2020

@underfin I tried the same code with defineAsyncComponent, it does not change the way the code is compiled and has the same problem with how import is converted to a require that lacks the default part of the import.

@underfin
Copy link
Member

underfin commented Sep 2, 2020

The compiled behavior is correct, defineAsyncComponent will handle default of module.

@tbgse
Copy link
Author

tbgse commented Sep 2, 2020

@underfin this does not seem to be the case, i might be doing something wrong?
Here is my router:

export default function (type) {
  const routerHistory = type === 'client' ? createWebHistory() : createMemoryHistory();

  return createRouter({
    history: routerHistory,
    routes: [
      { path: '/', component: Home, props: true },
      { path: '/a', component: defineAsyncComponent(() => import("./components/PageA.vue")), props: true },
      { path: '/b', component: defineAsyncComponent(() => import("./components/PageB.vue")), props: true },
    ]
  });
}

which is being compiled to:

function createRouter$1(type) {
  const routerHistory = type === "client" ? createWebHistory() : createMemoryHistory();
  return createRouter({
    history: routerHistory,
    routes: [
      {path: "/", component: script$2, props: true},
      {path: "/a", component: vue.defineAsyncComponent(() => Promise.resolve().then(function() {
        return require("./PageA.161ddab2.js");
      })), props: true},
      {path: "/b", component: vue.defineAsyncComponent(() => Promise.resolve().then(function() {
        return require("./PageB.f65a13cd.js");
      })), props: true}
    ]
  });
}

When running the server, i'm getting a warning and the page components do not render.

[Vue warn]: Component  is missing template or render function.

However, when i change return require("./PageB.f65a13cd.js"); to return require("./PageB.f65a13cd.js").default; everything works as expected. This is with the following versions of vue:

"vue": "^3.0.0-rc.9",
"@vue/compiler-sfc": "^3.0.0-rc.9",
"@vue/server-renderer": "^3.0.0-rc.9",

@underfin
Copy link
Member

underfin commented Sep 2, 2020

Can you provider your repo for me track this?

@tbgse
Copy link
Author

tbgse commented Sep 2, 2020

@underfin yes, i just set this up https://github.com/tbgse/vite-ssr-example

it's super basic, so i completely ignore hydration etc. in this example. But just by running npm run build and after that npm run serve you should be able to see the errors with the components not being found. Thanks for taking the time to look into this. You should be able to find the issue with the router in the build files (index.js) around line 1454. If you add .default to those requires and run npm run serve again, it will then correctly render the routes.

@underfin
Copy link
Member

underfin commented Sep 2, 2020

I try fix it,it is a bug with vue core. You can track here vuejs/core#2034

@tbgse
Copy link
Author

tbgse commented Sep 2, 2020

Awesome @underfin I'll keep a close eye on the PR in vue core.

@posva
Copy link
Contributor

posva commented Sep 2, 2020

You shouldn't use defineAsyncComponent with the router views, it's not necessary as they are resolved by the router navigation guards. So this is likely a bug in vue router missing some handling in the extractComponentsGuards function. It's handling it the same way as the previous vue-router, so it's weird. If someone could open an issue on vue-router-next with a boiled down repro, that would hepl

@tbgse
Copy link
Author

tbgse commented Sep 2, 2020

thanks for jumping in on this @posva with so many moving pieces (router, vue, vite) it was really hard for me to pinpoint where things were going wrong here. I posted a very minimal setup of the repo here: https://github.com/tbgse/vite-ssr-example and just updated it by removing defineAsyncComponent, so you can use it to reproduce the issue and debug the router.

Again this is only a problem when building in vite ssr mode. When you simply build for the client, everything works as expected.

@posva
Copy link
Contributor

posva commented Sep 2, 2020

Thanks, well for some reason the generated module is missing the __esModule property so we don't get the default.

@underfin I think there might be a bug in the transpilation on SSR: shouldn't the dynamic import output a require(...).default? Or add the __esModule property?
Checking for default in the resolved import should work but I'm not sure if it would break in some scenarios

@underfin
Copy link
Member

underfin commented Sep 3, 2020

Yeah.It look like lost __esModule property.

@underfin
Copy link
Member

underfin commented Sep 3, 2020

@tbgse .You can add blew code into vite.config.js as a workaround.

module.exports = {
  rollupInputOptions: {
    preserveEntrySignatures: 'strict',

  },
  rollupOutputOptions: {
    preserveModules: true,
  },
}

@tbgse
Copy link
Author

tbgse commented Sep 3, 2020

Thanks @underfin this works fine and hydration works like a charm with it too. It does create a massive amount of files compared to the default settings though, so i'm keeping a close eye on the issue you opened in rollup.

@tbgse
Copy link
Author

tbgse commented Sep 30, 2020

@underfin following up since rollup/plugins#552 seems to have been merged. I'm not 100% sure if this would need further adjustments in the vite config files (or internally?) or should it now work out of the box with the latest rollup version?

@tbgse
Copy link
Author

tbgse commented Oct 10, 2020

@underfin @antfu I could not get this to work with the latest @rollup/plugin-commonjs version, I also tried to change some of the options passed to the commonjs plugin in the createBaseRollupPlugins of vite but had no luck in getting this to work. Is there anything i can do to move this forward?

It seems like the original issue opened by underfin in the rollup repo was marked as resolved, so this should no longer be an upstream bug.

@underfin
Copy link
Member

I add comment with rollup/rollup#3760 (comment), please wait for answer.

@tbgse
Copy link
Author

tbgse commented Oct 24, 2020

@underfin it seems like a fix was merged in rollup rollup/rollup#3822

EDIT: it seems to work as expected now by installing the latest version of rollup and adding

    rollupOutputOptions: {
      namespaceToStringTag: true
    }

to the build config. Would it be possible to update the rollup dependency for vite?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug: upstream Bug in a dependency of Vite has workaround
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants