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

property access inside /* @__PURE__ */ makes an object not removed when minifying #2279

Closed
sapphi-red opened this issue May 29, 2022 · 7 comments

Comments

@sapphi-red
Copy link
Contributor

sapphi-red commented May 29, 2022

For example, BaseTransitionImpl is not removed from the output.

const TransitionHookValidator = [Function, Array];
const BaseTransitionImpl = {
  props: {
    mode: String,
    appear: Boolean,
    persisted: Boolean,
    onBeforeEnter: TransitionHookValidator,
    onEnter: TransitionHookValidator,
    onAfterEnter: TransitionHookValidator,
    onEnterCancelled: TransitionHookValidator,
    onBeforeLeave: TransitionHookValidator,
    onLeave: TransitionHookValidator,
    onAfterLeave: TransitionHookValidator,
    onLeaveCancelled: TransitionHookValidator,
    onBeforeAppear: TransitionHookValidator,
    onAppear: TransitionHookValidator,
    onAfterAppear: TransitionHookValidator,
    onAppearCancelled: TransitionHookValidator
  }
};
/* @__PURE__ */ Object.assign({}, BaseTransitionImpl.props);

esbuild repl

terser outputs the following code.

const o=[Function,Array];Boolean,Boolean;

Is this is not supported because it is difficult to support this without performance degration?

refs: https://github.com/vuejs/core/blob/dddbd96dfe69292cee401f72d2703e8fb3708a14/packages/runtime-dom/src/components/Transition.ts#L72-L77

@hyrious
Copy link

hyrious commented May 29, 2022

Yes, AFAIK property accessing is always treated as having side effects in esbuild since there might defined a custom getter. Tracking the pureness of an object prop is not in esbuild's goal.

There's still some way to hack around this, for example:

// as pure as "var a = 1"
var pure_value = /* @__PURE__ */ (() => obj.prop)()

@sapphi-red
Copy link
Contributor Author

I found a weird behavior.

  • BaseTransitionImpl gets removed.
    const a = /* @__PURE__ */ (() => BaseTransitionImpl.props)();
  • BaseTransitionImpl doesn't get removed.
    /* @__PURE__ */ (() => BaseTransitionImpl.props)();
    const a = /* @__PURE__ */ Object.assign({}, BaseTransitionImpl.props);
    /* @__PURE__ */ Object.assign({}, BaseTransitionImpl.props);

Is there any difference between these?

@hyrious
Copy link

hyrious commented May 29, 2022

There's a weird case to use --tree-shaking and --minify together:

In:

/* @__PURE__ */ (() => a.b)();
var c = /* @__PURE__ */ (() => d.e)();

Out:

a.b; // won't be here if removed --minify-syntax

See in repl.

So this might be a hidden bug in esbuild.

@hyrious
Copy link

hyrious commented May 29, 2022

Is there any difference between these?

The first 2 cases should be tree-shaked. This is because /* @__PURE__ */ is marking the function call to return a pure value. Which means:

var a = /* @__PURE__ */ foo(whatever)

The annotation does nothing about the things in braces. So if they have side effects, they are kept.

@sapphi-red
Copy link
Contributor Author

That makes sense.

So here it should be /* @__PURE__ */ (() => Object.assign({}, BaseTransitionImpl.props))();? (though it does not work currently as you pointed out #2279 (comment))

@evanw
Copy link
Owner

evanw commented May 29, 2022

Thanks for the report. This is just an oversight. Looks like I missed a break in fccd1c4 when I added the additional case for minifying unused IIFEs. That caused the code that minifies /* @__PURE__ */ (() => x)() into nothing to be overridden by the code that minifies (() => x)() into x.

I'll fix this in the next release. If you want to work around this in the meantime, you can use /* @__PURE__ */ (() => x)(0) instead of /* @__PURE__ */ (() => x)() since this optimization currently only applies if there are zero arguments.

@evanw evanw closed this as completed in fb73a30 May 29, 2022
@sapphi-red
Copy link
Contributor Author

Thank you!

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

No branches or pull requests

3 participants