vuejs / core

🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
https://vuejs.org/
MIT License
47.58k stars 8.31k forks source link

[compiler-sfc] Render function missing when default export is not accessed, inconsistent behavior between HMR and build #9011

Open zhangyx1998 opened 1 year ago

zhangyx1998 commented 1 year ago

Vue version

latest

Link to minimal reproduction

https://github.com/zhangyx1998/vite-plugin-vue-issue-reproduction

This link is correct, it says vite-plugin-vue because I originally submitted the issue there. See additional comments below for details.

Steps to reproduce

  1. npm install
  2. npm run dev will show all 3 components.
  3. npm run preview will only show component A and C.
  4. Use developer tools to see the components logged to console. You should notice the missing render function.

P.S. Screenshots are shown in README

What is expected?

The most intuitive behavior I would expect is compiler-sfc automatically inject compiled render function into whichever object that is wrapped with defineComponent(). Regardless of whether and how the component is exported. In this way more flexibility would be given to the developer.

What is actually happening?

However, in the current codebase, the facade module generated by compiler-sfc will not get a chance to run if the default exported variable is not accessed.

Due to the tree-shaking process, even if the default export was imported by another module, it will still not work if the imported variable was not read. This can be shown by commenting out this line in the provided reproduction snippet.

System Info

Not relevant

Any additional comments?

This issue was originally submitted under vitejs/plugin-vue. But as I dug deeper into the build process, I found that my issue was not introduced by the plugin. So I closed that issue and re-submitted it here.

Below is a copy of the original issue in which I wrote a code snippet to justify my use case

Describe the bug

The render function will NOT be injected in any of the conditions described below.

To make sense of my use case

I am trying to wrap some of my components using a "factory method". The factory method will return a callback function that injects some standard methods, variables and emits into wrapped components and then render the component in an enclosing window.

Here is a minimal example of my factory method:

function WindowFactory(component) {
    // Extract setup and emits from the original component
    const {
        setup = () => {},
        emits = [],
        ...others
    } = component;
    // Returns a callback to launch the window
    return function launch(...args) {
        // "Injected" component is prepared here
        const windowContent = {
            // This is the "hijacked" setup method
            setup() {
                return {
                    injectedMethod() {},
                    // Arguments are passed to the original setup method
                    ...setup(...args),
                };
            },
            // Mix injected emits into the component's original emits
            emits: ['injectedEmits', ...emits],
            // Bring back remainder of the component
            ...others
        };
        // Launch the window, and return a promise that resolves when the window is closed
        return new Promise((resolve, reject) => { /* ... */ });
    };
}

A demonstrative example that works in Vite HMR but fails in distribution:

<template> ... </template>

<script>
import { defineComponent } from 'vue';
import WindowFactory from '...';
// The component to be rendered inside a window
const component = defineComponent({ /* ... */ });
// This is the real export that will be called by other code
export const launchWindow = WindowFactory(component);
// Tell compiler where to inject render function
export default component;
</script>

The above function launchWindow, when invoked inside a bundled distribution, will not produce any error, and will neither render anything.

My exploration to this problem

After some experiment, I found out the following facts:

  1. The render function would NOT exist if I do not export it as default.

    // This dose NOT work:
    export const component = defineComponent({ /* ... */ });
    export const launchWindow = WindowFactory(component);
    // ('render' in component) === false
    console.log(component)
  2. Even if I export the component as default, it will NOT have render function when I do not access it in other contexts.

    // example.vue
    // This dose NOT work:
    export const component = defineComponent({ /* ... */ });
    export const launchWindow = WindowFactory(component);
    export default component;
    // ('render' in component) === false
    console.log(component)
  3. Accessing named exports (export const ...) from outside the SFC will still NOT produce the render function.

    // another_file.js
    // This dose NOT work:
    import { component } from './example.vue';
    // ('render' in component) === false
    console.log(component)
  4. As long as the default exported component is accessed OUTSIDE its SFC context (example.vue), it will get a render function.

    // yet_another_file.js
    // This works:
    import component from './example.vue';
    // ('render' in component) === true
    console.log(component)

    And after accessing the default export component, the in-context component will also magically receive a render function.

Real-world issue impact

This issue was found while developing a window framework for a web app.

  • The problematic code can be found in Random Student PickerCommit ae16206.

    To reproduce the issue:

    # git clone https://github.com/zhangyx1998/random-student-picker.git && cd random-student-picker
    git checkout ae1620641dba875b0ac3e9988e82875f41fad720;
    SRV_PORT=8080 make demo;

    The project will be served at local port 8080 or the port of your select.

  • A sample fix can be found in Random Student PickerCommit eeeb835

    To view the desired behavior:

    # git clone https://github.com/zhangyx1998/random-student-picker.git && cd random-student-picker
    git checkout eeeb8356149bda470b3c3c3ac50bcec200705792;
    SRV_PORT=8080 make demo;

    The project will be served at local port 8080 or the port of your select.

Problematic Desired
Problematic Screenshot Desired Screenshot

I would have to create separate files for functionalities that serve a single purpose. It works but also makes stuff scatter around. I have to maintain multiple files when I want to make changes to a single feature.

zhangyx1998 commented 1 year ago

Update

I further investigated the problem with the help of vite-plugin-inspect. I have identified the exact cause of this problem:

@vitejs/plugin-vue intentionally adds a /*#__PURE__*/ mark for the rewritten default!

The actual code that did this can be found in packages/plugin-vue/src/main.ts.

Transformed Vue SFC (raw file available at reproduction):

import { defineComponent } from 'vue';
export const component = defineComponent({
  setup(message) {
    return { message };
  }
});
export const getComponentB = (message) => {
  const { setup, ...others } = component;
  return defineComponent({
    setup() {
      return { ...setup(message) };
    },
    ...others
  });
};

// Injected by vite-plugin-vue transform

const _sfc_main = component;

import { toDisplayString as _toDisplayString, openBlock as _openBlock, createElementBlock as _createElementBlock } from "vue"

function _sfc_render(_ctx, _cache, $props, $setup, $data, $options) {
  return (_openBlock(), _createElementBlock("span", null, _toDisplayString(_ctx.message), 1))
}

import _export_sfc from 'plugin-vue:export-helper'
export default /*#__PURE__*/_export_sfc(_sfc_main, [['render',_sfc_render]])

After removing the /*#__PURE__*/ mark, the issue disappears!

image

Is this a bug or a feature?

I did not close this issue because the implementation in @vitejs/plugin-vue is somehow consistent with the behavior of the 'facade module' explanation presented in (compiler-sfc/README).

I did a lot of search and found out that there has not been any complaints on this issue, so I am not sure if my use case should actually be supported at all.

Please leave some comment on this, thanks!

zhangyx1998 commented 1 year ago

Attempting to fix

I found that removing /*#__PURE__*/ will fail many tests, since everything in attachedProps will be considered as referenced and will not shake away.

Take the above code (Transformed Vue SFC) for example, my objective is to allow tree shaking when and only when both component and _sfc_main are not referenced. Whatever tag we add to _sfc_main will not be able to take effect on component.

That said, one feasible approach would be directly manipulating the AST to rewrite defineComponent(...args) to defineComponent(...args, ...attachedProps).

To cover those who do not wrap their component in defineComponent(), a compile-time flag could be used to indicate if defineComponent() exists in context, and fall back to rewriting default export if not.

Again, please leave some comments!

zhangyx1998 commented 1 year ago

Proof of concept completed

A commit (ed6d312) is pushed to my forked of "vite-plugin-vue". In this commit, I demonstrated the idea that I described above.

Although the changes are made in the plugin repository, I believe they actually belongs to the compileScript module in compiler-sfc package. Which is why I keep updating this issue. Since '@babel/parse' and AST traverse should not be exposed to the plugin.

Sample Compile Result

More possibilities could be enabled with this change

Besides the factory function usage I mentioned above, this change can potentially enable multiple component implementation sharing the same template and styleSheet. Since export default is no longer required to inject props such as render functions (but will still be working as a fall back option).

This might be especially useful when user want to make serval components that look alike but function slightly differently.

<template> ... </template>

<script>
export const component_1 = defineComponent({
    // ... logic for component 1
})

export const component_2 = defineComponent({
    // ... logic for component 2
})
</script>

The concept works, but I need help pushing it further!

I have been talking to myself on this issue for several days. I devoted significant effort and time on this. I want to make it mean something.

@sxzz @yyx990803 Commit messages show that you are likely the primary contributors to the related modules. May I directly ask for your opinions?