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

V7 regression: class props + class syntax + arrow fn + closed over var #8648

Open
haf opened this Issue Sep 7, 2018 · 3 comments

Comments

Projects
None yet
3 participants
@haf

haf commented Sep 7, 2018

v7 Regression

When returning class declaration from arrow function, when using class properties plugin, and when the closure created by the arrow function includes a constant variable, babel goes down.

This was encountered in a v6 -> v7 upgrade. It's the only HoC that closes over variable and also uses the class declaration syntax, hence only problems in this one file.

Even if I rewrite this not to use arrow functions like so https://reactjs.org/docs/higher-order-components.html I get a crash.

ERROR in ./app/components/modal/index.js
Module build failed (from ./node_modules/babel-loader/lib/index.js):
SyntaxError: code/app/components/modal/index.js: Missing class properties transform. (This is an error on an internal node. Probably an internal error.)
    at File.buildCodeFrameError (code/node_modules/@babel/core/lib/transformation/file/file.js:259:12)
    at NodePath.buildCodeFrameError (code/node_modules/@babel/traverse/lib/path/index.js:157:21)
    at pushBody (code/node_modules/@babel/plugin-transform-classes/lib/transformClass.js:231:20)
    at buildBody (code/node_modules/@babel/plugin-transform-classes/lib/transformClass.js:205:5)
    at classTransformer (code/node_modules/@babel/plugin-transform-classes/lib/transformClass.js:610:5)
    at transformClass (code/node_modules/@babel/plugin-transform-classes/lib/transformClass.js:646:10)
    at PluginPass.ClassExpression (code/node_modules/@babel/plugin-transform-classes/lib/index.js:110:54)
    at newFn (code/node_modules/@babel/traverse/lib/visitors.js:193:21)
    at NodePath._call (code/node_modules/@babel/traverse/lib/path/context.js:53:20)
    at NodePath.call (code/node_modules/@babel/traverse/lib/path/context.js:40:17)

Describe the regression

Going from v6 -> v7, a single file in our large JS-code repo stops compiling:

Input Code

// @flow
import * as React from 'react';
import classNames from 'classnames';
export type ModalProps =
  { isOpen: boolean,
    onClose?: void => void,
    wideModal?: boolean
  }
type ModalState =
  { isOpen: boolean }
const modal = (Inner: any) =>
  class ModalWrapper extends React.Component<ModalProps, ModalState> {
    state = {
      isOpen: false,
    }
    closeModal = () => {
      this.setState({ isOpen: false });
      if (typeof this.props.onClose === 'function') {
        this.props.onClose();
      }
    }
    componentWillReceiveProps(nextProps: ModalProps) {
      this.setState({ isOpen: nextProps.isOpen })
    }
    render() {
      const { isOpen, onClose, wideModal, ...rest } = this.props;
      const modalClasses = classNames({ 'wide-modal': !!wideModal}, 'modal');
      return this.state.isOpen
        ? <div className={modalClasses} tabIndex='-1' role='dialog' ref='modal'>
            <div className="modal-body">
              <div className="modal-inner">
                <div className="form-wrapper">
                  <div className="form-wrapper-inner">
                    <button onClick={this.closeModal} className="modal-close-btn"></button>
                    <Inner {...rest} />
                  </div>
                </div>
              </div>
            </div>
          </div>
        : null
    }
  }
export default modal;

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

{
  "env": {
  // snip
    "development": {
      "presets": [
        "@babel/preset-flow",
        "@babel/preset-react",
        [
          "@babel/preset-env",
          { "modules": false }
        ]
      ],
      "plugins": [
        "@babel/plugin-proposal-class-properties",
        "@babel/plugin-transform-react-jsx-source",
        "@babel/plugin-transform-react-jsx-self",
        "flow-react-proptypes",
        [
          "react-intl",
          {
            "messagesDir": "./build/messages"
          }
        ]
      ]
    },
  },
 // snip
}
$ cat package.json | grep babel
    "@babel/cli": "^7.0.0",
    "@babel/core": "^7.0.0",
    "@babel/node": "^7.0.0",
    "@babel/plugin-proposal-class-properties": "^7.0.0",
    "@babel/plugin-proposal-decorators": "^7.0.0",
    "@babel/plugin-proposal-do-expressions": "^7.0.0",
    "@babel/plugin-proposal-export-default-from": "^7.0.0",
    "@babel/plugin-proposal-export-namespace-from": "^7.0.0",
    "@babel/plugin-proposal-function-bind": "^7.0.0",
    "@babel/plugin-proposal-function-sent": "^7.0.0",
    "@babel/plugin-proposal-json-strings": "^7.0.0",
    "@babel/plugin-proposal-logical-assignment-operators": "^7.0.0",
    "@babel/plugin-proposal-nullish-coalescing-operator": "^7.0.0",
    "@babel/plugin-proposal-numeric-separator": "^7.0.0",
    "@babel/plugin-proposal-optional-chaining": "^7.0.0",
    "@babel/plugin-proposal-pipeline-operator": "^7.0.0",
    "@babel/plugin-proposal-throw-expressions": "^7.0.0",
    "@babel/plugin-syntax-dynamic-import": "^7.0.0",
    "@babel/plugin-syntax-import-meta": "^7.0.0",
    "@babel/plugin-transform-react-constant-elements": "^7.0.0",
    "@babel/plugin-transform-react-jsx-source": "^7.0.0",
    "@babel/plugin-transform-runtime": "^7.0.0",
    "@babel/polyfill": "^7.0.0",
    "@babel/preset-env": "^7.0.0",
    "@babel/preset-flow": "^7.0.0",
    "@babel/preset-react": "^7.0.0",
    "@babel/runtime": "^7.0.0",
    "babel-eslint": "^9.0.0",
    "babel-loader": "^8.0.0",
    "babel-plugin-flow-react-proptypes": "^24.1.0",
    "babel-plugin-react-intl": "^2.3.1",
    "babel-plugin-react-transform": "^3.0.0",
    "babel-plugin-webpack-alias": "^2.1.2",
    "babel-preset-minify": "^0.4.3",
    "eslint-plugin-babel": "^5.1.0",

Expected behavior/code
A clear and concise description of what you expected to happen (or code).

Environment

  • Node/npm version: 8 LTS
  • OS: macOS 10.13.6
  • Monorepo huh?
  • How you are using Babel: "build:dev": "webpack --config webpack.dev.js --reporter basic", with webpack-command.
@babel-bot

This comment has been minimized.

Show comment
Hide comment
@babel-bot

babel-bot Sep 7, 2018

Collaborator

Hey @haf! 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 7, 2018

Hey @haf! 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.

@haf haf changed the title from Return class declaration from arrow function, when using class properties plugin crashes v6 -> v7 upgrade to class props + class syntax + arrow fn Sep 7, 2018

@haf haf changed the title from class props + class syntax + arrow fn to V7 regression: class props + class syntax + arrow fn + closed over var Sep 7, 2018

@nicolo-ribaudo

This comment has been minimized.

Show comment
Hide comment
@nicolo-ribaudo

nicolo-ribaudo Sep 9, 2018

Member

It looks like babel-plugin-flow-react-proptypes (https://github.com/brigand/babel-plugin-flow-react-proptypes/blob/9521fb1030a50b4df45e7025d79ff81293ecf4fe/package.json#L58) and babel-plugin-react-intl (https://github.com/yahoo/babel-plugin-react-intl/blob/1b571e7fb47c7780622c4751bf8833f81acd2a73/package.json#L18) still use Babel 6. Can you reproduce this bug without them? If not, it's because they don't support Babel 7 yet.

Member

nicolo-ribaudo commented Sep 9, 2018

It looks like babel-plugin-flow-react-proptypes (https://github.com/brigand/babel-plugin-flow-react-proptypes/blob/9521fb1030a50b4df45e7025d79ff81293ecf4fe/package.json#L58) and babel-plugin-react-intl (https://github.com/yahoo/babel-plugin-react-intl/blob/1b571e7fb47c7780622c4751bf8833f81acd2a73/package.json#L18) still use Babel 6. Can you reproduce this bug without them? If not, it's because they don't support Babel 7 yet.

@haf

This comment has been minimized.

Show comment
Hide comment
@haf

haf Sep 10, 2018

brigand/babel-plugin-flow-react-proptypes#150 (comment) seems to indicate it supports v7 and v7 made react-intl work better for us (actually capturing all defaults).

Since I've now worked around by hoisting the value to props, I'm not going to spend more time on this issue. You have the code and deps for a repro, otherwise, feel free to close this issue. Also, thank you for the investigation you did and your reply.

haf commented Sep 10, 2018

brigand/babel-plugin-flow-react-proptypes#150 (comment) seems to indicate it supports v7 and v7 made react-intl work better for us (actually capturing all defaults).

Since I've now worked around by hoisting the value to props, I'm not going to spend more time on this issue. You have the code and deps for a repro, otherwise, feel free to close this issue. Also, thank you for the investigation you did and your reply.

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