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

Usage of `break` inside a `case` of a `switch...case` statement is transpiled to `return "break"; which breaks the outer `for...of` loop iteration #8709

Open
franktopel opened this Issue Sep 14, 2018 · 7 comments

Comments

Projects
None yet
4 participants
@franktopel

franktopel commented Sep 14, 2018

Bug Report

Current Behavior
Usage of break inside a case of a switch...case statement is transpiled to return "break"; instead of break; if the case instructions are wrapped in a block. This exits the surrounding for...of loop iteration, making the code following the switch...case unreachable.

Input Code

document.addEventListener('DOMContentLoaded', function() {
  this.initializers = {
    dependentControls() {
      const dependers = [...document.querySelectorAll('[data-depends-on]')]
      for (const depender of dependers) {
        if (depender.closest('form').classList.contains('report-view')) {
          return
        }
        let dependency = document.getElementById(depender.dataset.dependsOn)
        if (dependency) {
          let dependencyDetails = { prop: 'checked', state: false }
          switch (dependency.type) {
            case 'checkbox': {
              dependencyDetails.prop = 'checked'
              dependencyDetails.state = false
              break
            }
            case 'text': {
              dependencyDetails.prop = 'value'
              dependencyDetails.state = ''
              break
            }
            default:
          }
          console.log('switch...end') // this line is never reached
          depender.disabled = !dependency[dependencyDetails.prop]
          dependency.addEventListener('change', () => {
            depender.disabled = dependency[dependencyDetails.prop] === dependencyDetails.state
          })
        }
      }
      console.log('for...end')
    }
  }
  /* more code, not relevant here */
});

Output code

dependentControls() {
  var dependers = _toConsumableArray(document.querySelectorAll('[data-depends-on]'));
  
  var _iteratorNormalCompletion5 = true;
  var _didIteratorError5 = false;
  var _iteratorError5 = undefined;

  try {
    var _loop5 = function _loop5() {
      var depender = _step5.value;

      var dependency = document.getElementById(depender.dataset.dependsOn);

      if (dependency) {
        var dependencyDetails = {
          prop: 'checked',
          state: false
        };

        switch (dependency.type) {
          case 'checkbox':
            {
              dependencyDetails.prop = 'checked';
              dependencyDetails.state = false;
              return "break";
            }

          case 'text':
            {
              dependencyDetails.prop = 'value';
              dependencyDetails.state = '';
              return "break";
            }

          default:
        }

        console.log("switch...end"); // this line is never reached

        depender.disabled = !dependency[dependencyDetails.prop];
        dependency.addEventListener('change', function() {
          depender.disabled = dependency[dependencyDetails.prop] === dependencyDetails.state;
        });
      }
    };

    _loop4: for (var _iterator5 = dependers[Symbol.iterator](), _step5; !(_iteratorNormalCompletion5 = (_step5 = _iterator5.next()).done); _iteratorNormalCompletion5 = true) {
      var _ret2 = _loop5();

      switch (_ret2) {
        case "break":
          break _loop4;

        default:
          if (_typeof(_ret2) === "object") return _ret2.v;
      }
    }
  } catch (err) {
    _didIteratorError5 = true;
    _iteratorError5 = err;
  } finally {
    try {
      if (!_iteratorNormalCompletion5 && _iterator5.return != null) {
        _iterator5.return();
      }
    } finally {
      if (_didIteratorError5) {
        throw _iteratorError5;
      }
    }
  }

  console.log("for...end");
}

Sample HTML this is meant to operate on:

<div>
  <input type="checkbox" id="fooBar" />
  <label for="fooBar">dependency</label>
</div>
<hr />
<div>
  <label for="fooBaz">depender</label>
  <input type="text" id="fooBaz" data-depends-on="fooBar" value="depender" />
</div>

Expected behavior/code
Using break inside switch...case transpiles to break; and not return "break";.

Babel Configuration (.babelrc, package.json, cli command)

.babelrc

{
  "presets": [
    [ "@babel/preset-env", {
      "targets": {
        "browsers": [ "last 2 versions", "ie >= 11" ]
      }
    }]
  ]
}

package.json - dependencies

"devDependencies": {
    "@babel/core": "^7.0.1",
    "@babel/preset-env": "^7.0.0",
    "autoprefixer": "^9.1.5",
    "babel-eslint": "^9.0.0",
    "babel-loader": "^8.0.2",
    "babel-plugin-transform-runtime": "^6.23.0",
    "babel-runtime": "^6.26.0",
    "browser-sync": "^2.24.7",
    "css-mqpacker": "^7.0.0",
    "del": "^3.0.0",
    "eslint": "^5.5.0",
    "eslint-config-airbnb": "^17.1.0",
    "eslint-config-last": "0.0.5",
    "eslint-config-prettier": "^3.0.1",
    "eslint-formatter-pretty": "^1.1.0",
    "eslint-loader": "^2.1.0",
    "eslint-plugin-import": "^2.14.0",
    "eslint-plugin-prettier": "^2.6.2",
    "fontello-cli": "^0.4.0",
    "fontello-webpack-plugin": "^0.4.8",
    "gulp": "^3.9.1",
    "gulp-changed": "^3.2.0",
    "gulp-compile-handlebars": "^0.6.1",
    "gulp-consolidate": "^0.2.0",
    "gulp-filter": "^5.1.0",
    "gulp-front-matter": "^1.3.0",
    "gulp-git": "^2.8.0",
    "gulp-handlebars-file-include": "^1.0.0",
    "gulp-iconfont": "^10.0.1",
    "gulp-if": "^2.0.0",
    "gulp-include": "^2.0.3",
    "gulp-mustache": "^3.0.1",
    "gulp-notify": "^3.2.0",
    "gulp-nunjucks-render": "^2.0.0",
    "gulp-plumber": "^1.0.1",
    "gulp-postcss": "^8.0.0",
    "gulp-prettify": "^0.5.0",
    "gulp-rename": "^1.4.0",
    "gulp-sass": "^4.0.1",
    "gulp-sourcemaps": "^2.6.4",
    "gulp-svgmin": "^2.0.0",
    "gulp-util": "^3.0.7",
    "handlebars": "^4.0.12",
    "handlebars-partial-file": "^1.0.0",
    "html-webpack-plugin": "^3.2.0",
    "htmlhint": "^0.10.1",
    "husky": "^0.14.3",
    "json5-loader": "^1.0.1",
    "lint-staged": "^7.2.2",
    "lodash": "^4.17.10",
    "lost": "^8.3.0",
    "mustache": "^2.3.2",
    "node-sass": "^4.9.3",
    "postcss-csso": "^3.0.0",
    "prettier": "^1.14.2",
    "require-dir": "^1.0.0",
    "require-yaml": "^0.0.1",
    "run-sequence": "^2.2.1",
    "stylelint": "^9.5.0",
    "stylelint-selector-bem-pattern": "^2.0.0",
    "webpack": "^4.18.0",
    "webpack-bundle-analyzer": "^2.13.1",
    "webpack-dashboard": "^2.0.0"
  },
  "dependencies": {
    "@babel/polyfill": "^7.0.0",
    "axios": "^0.18.0",
    "bootstrap": "^4.1.3",
    "datatables.net-bs4": "^1.10.19",
    "datatables.net-fixedcolumns-bs4": "^3.2.6",
    "datatables.net-plugins": "^1.10.18",
    "datatables.net-responsive-bs4": "^2.2.3",
    "ekko-lightbox": "5.2.0",
    "element-closest": "^2.0.2",
    "jquery": "^3.3.1",
    "jquery.localscroll": "^2.0.0",
    "jquery.scrollto": "^2.1.2",
    "nodelist-foreach-polyfill": "^1.2.0",
    "popper.js": "^1.14.4"
  }

webpack.config.js module rule

        {
          test: /\.js$/,
          loader: 'babel-loader',
          exclude: [
            path.resolve(__dirname, 'node_modules'),
          ],
        }

Environment

  • Babel version(s): 7.0.1
  • Node/npm version: Node 8.11.4, npm 6.4.1
  • OS: MacOS 10.13.6, Arch Linux
  • How you are using Babel: babel-loader via webpack, 8.0.2

Possible Solution

Additional context/Screenshots
I've asked on Stackoverflow about it: https://stackoverflow.com/questions/52310227/using-switch-case-the-break-inside-a-case-ends-the-outer-for-of-loop-iteration

@babel-bot

This comment has been minimized.

Show comment
Hide comment
@babel-bot

babel-bot Sep 14, 2018

Collaborator

Hey @franktopel! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

Collaborator

babel-bot commented Sep 14, 2018

Hey @franktopel! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

@franktopel

This comment has been minimized.

Show comment
Hide comment
@franktopel

franktopel Sep 14, 2018

It seems this has been reported in April already: #7765, with noone taking care so far seemingly. :(

franktopel commented Sep 14, 2018

It seems this has been reported in April already: #7765, with noone taking care so far seemingly. :(

@franktopel franktopel changed the title from Usage of `break` inside a `case` of a `switch...case` statement is transpiled to `return "break";` instead of `break;`. to Usage of `break` inside a `case` of a `switch...case` statement is transpiled to `return "break"; which breaks the outer `for...of` loop iteration Sep 14, 2018

@franktopel

This comment has been minimized.

Show comment
Hide comment
@franktopel

franktopel Sep 14, 2018

Interestingly enough, the online REPL doesn't transpile break to return "break". What setting/configuration would cause that behaviour?

franktopel commented Sep 14, 2018

Interestingly enough, the online REPL doesn't transpile break to return "break". What setting/configuration would cause that behaviour?

@nicolo-ribaudo

This comment has been minimized.

Show comment
Hide comment
@nicolo-ribaudo

nicolo-ribaudo Sep 14, 2018

Member

It seems this has been reported in April already: #7765, with noone taking care so far seemingly. :(

Do you want to open a PR? 🙂

Interestingly enough, the online REPL doesn't transpile break to return "break". What setting/configuration would cause that behaviour?

Yeah, the REPL is still using Babel 6. You can test Babel 7 at https://babeljs.io/repl/build/master

Member

nicolo-ribaudo commented Sep 14, 2018

It seems this has been reported in April already: #7765, with noone taking care so far seemingly. :(

Do you want to open a PR? 🙂

Interestingly enough, the online REPL doesn't transpile break to return "break". What setting/configuration would cause that behaviour?

Yeah, the REPL is still using Babel 6. You can test Babel 7 at https://babeljs.io/repl/build/master

@franktopel

This comment has been minimized.

Show comment
Hide comment
@franktopel

franktopel Sep 15, 2018

Temporary solution:

Don't wrap case instructions in a new block context {} until this issue has been covered.

It only goes wrong if the case instructions are wrapped in a block.

This works as expected:

switch(a) {
  case 1:
    /** some code **
    break
  default:
}

This creates the problem:

switch(a) {
  case 1: {
    /** some code **
    break
  }
  default: {}
}

franktopel commented Sep 15, 2018

Temporary solution:

Don't wrap case instructions in a new block context {} until this issue has been covered.

It only goes wrong if the case instructions are wrapped in a block.

This works as expected:

switch(a) {
  case 1:
    /** some code **
    break
  default:
}

This creates the problem:

switch(a) {
  case 1: {
    /** some code **
    break
  }
  default: {}
}

@franktopel franktopel closed this Sep 15, 2018

@franktopel

This comment has been minimized.

Show comment
Hide comment
@franktopel

franktopel Sep 15, 2018

Accidentally closed. Sorry

franktopel commented Sep 15, 2018

Accidentally closed. Sorry

@franktopel franktopel reopened this Sep 15, 2018

@DBosley

This comment has been minimized.

Show comment
Hide comment
@DBosley

DBosley Sep 21, 2018

I'm having this issue too. Disappointed that it still hasn't been resolved if it was reported in April.
I'd love to make a PR for it, but I've got no clue where to even start looking for the problem in the codebase.

DBosley commented Sep 21, 2018

I'm having this issue too. Disappointed that it still hasn't been resolved if it was reported in April.
I'd love to make a PR for it, but I've got no clue where to even start looking for the problem in the codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment