Skip to content

Conversation

cuixiaorui
Copy link
Contributor

Change to export from @vue/shared

No repeat declaration

@lmiller1990
Copy link
Member

lmiller1990 commented Jul 13, 2020

I think we decided to avoid the @vue/shared dependency and copy paste instead to simplify the bundle when used as an es module, for example. Maybe @JessicaSachs has some more context on this (mentioned here, too)? I think we talked about this one before. No dependencies is the best number of dependencies, more often than not.

@cuixiaorui
Copy link
Contributor Author

cuixiaorui commented Jul 13, 2020

I did some tests on this problem

After packing with yarn build

 private get hasMultipleRoots(): boolean {
    // if the subtree is an array of children, we have multiple root nodes
    return this.vm.$.subTree.shapeFlag === ShapeFlags.ARRAY_CHILDREN
  }

After the build

Object.defineProperty(VueWrapper.prototype, "hasMultipleRoots", {
            get: function () {
                // if the subtree is an array of children, we have multiple root nodes
                return this.vm.$.subTree.shapeFlag === 16 /* ARRAY_CHILDREN */;
            },
            enumerable: true,
            configurable: true
        });

Directly changes shapeflags.array_children to a number

With the rollup Tree Shaking feature, you don't need to worry about vue-test-utils.[xxx].js getting bigger

As far as package dependency is concerned, we've already had the user install the Vue, and @vue/share is what will exist once the Vue is installed, so should we really worry about this?

@lmiller1990 lmiller1990 self-requested a review July 14, 2020 01:43
@lmiller1990
Copy link
Member

I think you are correct here, I did not realize it would be able to do this. Are we able to use this in a browser (and with vite)? If this is the case, we should definitely merge this PR, it's a great improvement. I'll give it a test this evening. Thanks!

Copy link
Member

@lmiller1990 lmiller1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, this is not a problem. Great!

@lmiller1990 lmiller1990 merged commit e21852e into vuejs:master Jul 15, 2020
@cuixiaorui cuixiaorui deleted the remove-shapeFlags branch July 15, 2020 11:11
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