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

CRA 2 browserslist build can generate invalid code #4665

Closed
jazeee opened this issue Jun 22, 2018 · 4 comments
Closed

CRA 2 browserslist build can generate invalid code #4665

jazeee opened this issue Jun 22, 2018 · 4 comments

Comments

@jazeee
Copy link

jazeee commented Jun 22, 2018

Is this a bug report?

Yes

Did you try recovering your dependencies?

Yes
I created a minimal repro using CRA2 "react-scripts": "2.0.0-next.3e165448"

Which terms did you search for in User Guide?

browserslist, bad build, yarn build

Environment

npx create-react-app --info

Environment:
  OS:  Linux 4.13
  Node:  8.11.2
  Yarn:  1.7.0
  npm:  5.6.0
  Watchman:  Not Found
  Xcode:  N/A
  Android Studio:  Not Found

Packages: (wanted => installed)
  react: ^16.4.1 => 16.4.1
  react-dom: ^16.4.1 => 16.4.1
  react-scripts: 2.0.0-next.3e165448 => 2.0.0-next.3e165448

Steps to Reproduce

Clone https://github.com/jazeee/cra-build-bug-demo
Repro in Demo app:

  1. yarn start
  2. Click on the red div element. Observe an alert saying Works.
  3. yarn build
  4. node_modules/.bin/serve build/
  5. Click on the red div element.
    Observe an alert saying TypeError: Assignment to constant variable.

Expected Behavior

Expect Works
Also, can see the compiled code has a problem:

			const t = e.data
			  , n = e.selectedRows
				//...
					onClick: ()=>{
						try {
							n = n, // This is being reassigned...
				//... Stripped for clarity

For reference, can make the minified code beautiful using:
yarn build && js-beautify build/static/js/main.*.js

Actual Behavior

Observe an alert saying TypeError: Assignment to constant variable.

Reproducible Demo

Clone https://github.com/jazeee/cra-build-bug-demo

Works when using:

In package.json:

  "browserslist": {
    "development": [
      "last 1 chrome versions"
    ],
    "production": [
      ">1%",
      "last 4 versions",
      "Firefox ESR",
      "not ie < 11"
    ]
  }

Since this is using a "bleeding edge" build (last 1 browser version), feel free to consider this as minor. It probably is an issue, but it can wait...
Thanks much.

@Timer
Copy link
Contributor

Timer commented Jun 22, 2018

/cc @existentialism

@Timer Timer added this to the 2.0.0 milestone Jun 22, 2018
@bugzpodder
Copy link

bugzpodder commented Jun 23, 2018

Enabling @babel/plugin-transform-block-scoping will have resulted in valid code.
Disabling uglifyjs will also result in valid code.

@bugzpodder
Copy link

Before Uglify:

// CONCATENATED MODULE: ./src/checkbox-column.jsx
      const selectAll = (ids, selectedRows) => {
        // This is just an example, stripped down to repro the build issue. (Obviously, no longer really useful)
        ids.every(id => selectedRows.includes(id));
      };
      const getCheckboxColumn = selectionProps => {
        const data = selectionProps.data,
          selectedRows = selectionProps.selectedRows,
          idKey = selectionProps.idKey;
        const ids = data.map(
          (instance, index) => (idKey ? instance[idKey] : `${index}`)
        );
        return {
          Header: react_default.a.createElement(
            "div",
            {
              style: { width: "300px", height: "100px", background: "tomato" },
              onClick: () => {
                try {
                  selectAll(ids, selectedRows);
                  alert("Works");
                } catch (error) {
                  console.error(error);
                  alert(error);
                }
              }
            },
            "Click Me (Works for CRA `yarn start` Fails after build.)"
          )
        };
      };

After:

const s = e => {
        const t = e.data,
          n = e.selectedRows,
          r = e.idKey,
          o = t.map((e, t) => (r ? e[r] : `${t}`));
        return {
          Header: a.a.createElement(
            "div",
            {
              style: { width: "300px", height: "100px", background: "tomato" },
              onClick: () => {
                try {
                  (n = n), o.every(e => n.includes(e)), alert("Works");
                } catch (e) {
                  console.error(e), alert(e);
                }
              }
            },
            "Click Me (Works for CRA `yarn start` Fails after build.)"
          )
        };
      };

@bugzpodder
Copy link

I tried out terser and it seem to work better.
https://github.com/fabiosantoscode/terser#replacing-uglify-es-with-terser-in-a-project-using-yarn

@Timer Timer modified the milestones: 2.0.x, 2.0.0-final, 2.0.0 Sep 26, 2018
@Timer Timer closed this as completed Sep 27, 2018
@lock lock bot locked and limited conversation to collaborators Jan 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants