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

Top level variables not dead code eliminated #1183

Closed
jaydenseric opened this issue Apr 23, 2021 · 6 comments
Closed

Top level variables not dead code eliminated #1183

jaydenseric opened this issue Apr 23, 2021 · 6 comments

Comments

@jaydenseric
Copy link

jaydenseric commented Apr 23, 2021

Using bundle: true and minify: true, with an entry point containing only this results in the variable being eliminated:

const a = 1;

But this results in no elimination (assume b is a global):

const a = b;

It should be eliminated just the same.

A similar problem manifests when destructuring. This whole statement gets eliminated:

const { a } = {};

While this does not:

const { a } = b;

I think this bug is the root reason why useDebugValue destructured from require('react') is not eliminated as dead code in my bundle, even when it's use within the module is eliminated:

if (typeof process === 'object' && process.env.NODE_ENV !== 'production')
  useDebugValue(value);
@evanw
Copy link
Owner

evanw commented Apr 23, 2021

The reason is that const a = 1 is considered to have no side effects but const a = b is considered to have side effects. If b is not defined then attempting to access it will throw, which is a side effect. There may also be a getter declared on the global object for property b with arbitrary side effects. If b is a local variable such as var b; const a = b then esbuild should consider it to have no side effects. This is not a bug in esbuild.

@jaydenseric
Copy link
Author

jaydenseric commented Apr 23, 2021

Is there a way to declare purity, to allow dead code elimination?

E.g.

const a = /*#__PURE__*/ b;
const {
  /*#__PURE__*/ useDebugValue
} = /*#__PURE__*/ require('react');

@jaydenseric
Copy link
Author

At the very least, the aliasing done by the minifier could be eliminated:

- const { useDebugValue: a } = require('react');
+ const { useDebugValue } = require('react');

@evanw
Copy link
Owner

evanw commented May 14, 2021

Is there a way to declare purity, to allow dead code elimination?

Yes. Pure comments apply to call expressions, not to identifier expressions. So you could do this:

const a = /*#__PURE__*/ (() => b)();

jaydenseric added a commit to jaydenseric/graphql-react that referenced this issue May 19, 2021
@jaydenseric
Copy link
Author

@evanw I managed to avoid the original dilemma by not destructing require from react. That was fine because the whole const React = require('react') call doesn't need to be eliminated because there are still other react APIs (e.g. React.useCallback) being used from it. But how can you declare a require call is "pure", so the whole statement can be dead-code-eliminated?

I've tried making sure the required package has the package "sideEffects": false field, and I've tried:

const PropTypes = /*#__PURE__*/ require('prop-types');

Yet, with something like:

if (typeof process === 'object' && process.env.NODE_ENV !== 'production') {
  Foo.propTypes = {
    bar: PropTypes.bool,
  };
}

In the production bundle all the uses of PropTypes in the module are gone, but the require('prop-types') (minified, so using the single-letter require helper) is still needlessly there.

I'd raised a prop-types PR to add the package sideEffects field (facebook/prop-types#350) in the hope it would result in references to that package being entirely eliminated from my modules, but I'm struggling to get the desired result even when I hardcode that field within the package in node_modules and re-run esbuild.

@evanw
Copy link
Owner

evanw commented Feb 16, 2022

But how can you declare a require call is "pure", so the whole statement can be dead-code-eliminated?

My answer in #1183 (comment) was general-purpose. Replace b with whatever expression you want. So you can do this for example:

const PropTypes = /*#__PURE__*/ (() => require('prop-types'))();

I'm going to close issue this since a) there is already a way to mark top-level code as pure and b) the issue facebook/prop-types#350 about marking prop-types as side-effect free has already been fixed and shipped.

@evanw evanw closed this as completed Feb 16, 2022
denisp22 added a commit to denisp22/graphql-react that referenced this issue Jul 27, 2023
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

2 participants