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

Parser decorators plugin option decoratorsBeforeExport no longer defaulting #8562

Closed
jaydenseric opened this issue Aug 28, 2018 · 29 comments · Fixed by #8576
Closed

Parser decorators plugin option decoratorsBeforeExport no longer defaulting #8562

jaydenseric opened this issue Aug 28, 2018 · 29 comments · Fixed by #8576
Labels
i: docs outdated A closed issue/PR that is archived due to age. Recommended to make a new issue

Comments

@jaydenseric
Copy link

Bug Report

Current Behavior
In a recent v7 release (maybe even the final?), the parser decorators plugin option decoratorsBeforeExport no longer defaults as it used to.

const { parse } = require('@babel/core')

const result = parse(code, {
  parserOpts: {
    plugins: [
      'decorators'
    ]
  }
})

Results in an error:

/Users/jaydenseric/Sites/demo/node_modules/@babel/core/lib/transformation/normalize-file.js:209
    throw err;
    ^

Error: The 'decorators' plugin requires a 'decoratorsBeforeExport' option, whose value must be a boolean.

This is unexpected, as the decoratorsBeforeExport option is meant to default to false. Manually providing the same value as the supposed default works:

  const result = parse(code, {
    parserOpts: {
      plugins: [
-       'decorators'
+       ['decorators', { decoratorsBeforeExport: false }]
      ]
    }
  })

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

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

N/A.

Environment

  • Babel version(s): v7.0.0
  • Node/npm version: Node.js v10.9.0, npm v6.4.0
  • OS: macOS
  • Monorepo [e.g. yes/no/Lerna]: No
  • How you are using Babel: [e.g. cli, register, loader]: API

Possible Solution

Additional context/Screenshots

@babel-bot
Copy link
Collaborator

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

@loganfsmyth
Copy link
Member

This was a purposeful change in #8465 for now because the spec authors haven't settled on one or the other. To avoid us choosing the wrong default, we opted to require users to choose.

In the short term, I'd expect users transitioning from 6.x to use legacy: true anyway.

@jaydenseric
Copy link
Author

Maybe the docs should be updated to reflect that there is no longer a default. Encountered the issue when reinstalling node_modules for jsdoc-md here: https://github.com/jaydenseric/jsdoc-md/blob/v1.5.0/lib/jsdocCommentsFromCode.js#L16

@loganfsmyth
Copy link
Member

loganfsmyth commented Aug 28, 2018

Totally, happy to leave this open to we can address the docs. FYI, you may want to consider using 'decorators-legacy' since that is what was supported in 6.x.

Also, in the long run, since you're using babel/core's parse, you could pass it a filename and remove decorators entirely and rely on your users having a Babel config file that enables the proper decorators proposal on their side.

@the-simian
Copy link

the-simian commented Aug 28, 2018

I'm struggling with this right now, when I set this:

      [
        require('@babel/plugin-proposal-decorators'),
        { decoratorsBeforeExport: false, legacy: true }
      ],

OR

      [
        require('@babel/plugin-proposal-decorators'),
        { decoratorsBeforeExport: true, legacy: true }
      ],

For both I get the same error 'decoratorsBeforeExport' can't be used with legacy decorators.

if I remove the legacy flag, like this:

      [
        require('@babel/plugin-proposal-decorators'),
        { decoratorsBeforeExport: false }
      ],

I get "The new decorators proposal is not supported yet. You must pass the "legacy": true option to @babel/plugin-proposal-decorators".

if I remove the decoratorsBeforeExport entirely and just do the legacy flag,

[require('@babel/plugin-proposal-decorators'), { legacy: true }],

I am able to proceed...

However this does now cause errors with using decorators and then using export default. I'm aware the spec is 'still in flux' but what is the way to configure this where I can use decorators like

@something
export default class MyClass

Is this no longer able to be done?

@nicolo-ribaudo
Copy link
Member

Just using { legacy: true } should work. What error are you getting?

@the-simian
Copy link

the-simian commented Aug 29, 2018

Its not giving me a message like the others, just 'failed to compile'. when I change the files to

@something
class Thing
...
export default Thing;

that works. I'm not really sure what's wrong but in the previous beta (7.0.0-beta.49) this worked, and for me at least in 7 its not. What I mean is that code that had decorators preceding an export did not break, and I was able to specify decoratorsBeforeExport : true and everything was fine.

If you're telling me that's a config that works, its possible the error is in eslint or something. I mainly wanted to confirm that's the correct config, but I am getting breaks on situations where there are dercorators before export and I was not beforehand. Should that config be working on decorators before the export keyword ?

@nicolo-ribaudo

This comment has been minimized.

@nicolo-ribaudo
Copy link
Member

Sorry y'all, I was confused with another issue. Our parser has two diffrent plugins: decorators (which is the new decorators proposal) and decorators-legacy. If you want the old behavior, you should use decorators-legacy.

@the-simian
Copy link

the-simian commented Aug 29, 2018

@nicolo-ribaudo I'll answer your questions first:
I am indeed using @babel/plugin-proposal-decorators at 7.0.0. I was using this in the 7 beta as well.
I am using babel-eslint at 9.0.0 also.

I am a bit confused on what is meant about the new/old proposal. I have been using this 'new' proposal lib and only with the release of 7.0.0 did it break. It was working with beta-49+ as far as i can recall. I'm using and have been using the new behavior. However, I am not sure what the expectation right now in this proposal plugin should be. I was under the impression that there was no consensus yet on before or after export (such as in this thread) tc39/proposal-decorators#69 and that the plugin you've authored here allowed the enduser to configure, via a flag (decoratorsBeforeExport).

Regardless, rather than referencing these confusing threads with hundreds of back-and-forth comments, more directly I am asking :

  • with this plugin (@babel/plugin-proposal-decorators) can you do decorators like export default class Thing at all? you could in the beta version, was this removed?
  • if not, can you post a very very small snippet of a decorator that this plugin actually accepts so I understand 'code first' what should actually be written with this plugin ?
  • could this simply be a problem with babel-eslint @9 ? where it is indeed parsing, but somehow the linter is throwing anyways? (maybe there is non parity between the linter and parser?)

I've already converted my codebase to

@decorator
class Thing{}
export default Thing

I've been able to move past, but this was really time consuming, and to be honest I preferred the way it was beforehand.

Sorry to keep this going, but any answers you give help other folks besides myself reading this get some clarity. The decorators 'situation' is very confusing overall, even if you're studiously reading the TC-39 threads (most people aren't).

Thanks again for any replies, and I appreciate the effort on this project.

@nicolo-ribaudo
Copy link
Member

Sorry for the confusion, I get that old/new proposal doesn't mean anything for many people.

The "old" proposal is the one y'all are used to: it's the only one implemented in Babel and TypeScript. It's the one supported by babel-plugin-transform-decorators-legacy and @babel/plugin-transform-decorators, { legacy: true }. In @babel/parser it is decorators-legacy.

The "new" proposal is currently only implemented in @babel/parser (using the decorators plugin) and in babel-eslint, because it uses @babel/parser. We are implementing it (#7976), and also the TypeScript team is.

The decoratorsBeforeExport option is only meaningful for the "new" version of the proposal because, as you noted, there isn't consensus yet. You can't really use it in @babel/plugin-proposal-decorators because #7976 has not been merged yet, but you can use it in the @babel/parser's decorators plugin.


with this plugin (@babel/plugin-proposal-decorators) can you do decorators like export default class Thing at all? you could in the beta version, was this removed?

You still can, the old behavior hasn't changed. We only require the legacy option now:

const out = babel.transformSync(
  `
  @decorator
  export default class Foo {}
  `,
  {
    plugins: [
      [
        require("./packages/babel-plugin-proposal-decorators"),
        { legacy: true },
      ],
    ],
  }
);

console.log(out.code);

logs

var _class;

let Foo = decorator(_class = class Foo {}) || _class;

export { Foo as default };

could this simply be a problem with babel-eslint @9 ? where it is indeed parsing, but somehow the linter is throwing anyways? (maybe there is non parity between the linter and parser?)

In babel-eslint you have to opt-in to use the "old" proposal: in your eslint config, you should have something like this:

{
  parserOptions: {
    ecmaFeatures: {
      legacyDecorators: true
    }
  }
}

@jaydenseric
Copy link
Author

jaydenseric commented Aug 30, 2018

This issue should not have been closed, since the docs are still incorrect:

screen shot 2018-08-30 at 5 58 02 pm

The decoratorsBeforeExport option does not default to false.

@nicolo-ribaudo
Copy link
Member

Do you want to open a PR to babel/website?

@existentialism
Copy link
Member

existentialism commented Aug 30, 2018

Fixed:

image

Sorry for any confusion the docs caused, as you can imagine... lots of moving parts leading up to shipping 7!

@damianobarbati
Copy link

@the-simian I had you same problem, solved with @nicolo-ribaudo suggestion, change you eslint config:

    "eslintConfig": {
        "parser": "babel-eslint",
        "parserOptions": {
            "ecmaVersion": 2019,
            "sourceType": "module",
            "impliedStrict": true,
            "ecmaFeatures": {
                "jsx": true,
                "impliedStrict": true,
                "globalReturn": false,
                "experimentalObjectRestSpread": true,
                "legacyDecorators": true
            }
        },
        "env": {
            "browser": true,
            "serviceworker": true,
            "node": true,
            "jest": true,
            "es6": true
        },
        "plugins": [
            "react",
            "jest"
        ],
        "extends": [
            "eslint:recommended",
            "plugin:react/recommended",
            "plugin:jest/recommended"
        ],
        "rules": {
            "no-console": "off"
        }
    },

@the-simian
Copy link

I believe this was my problem thank you @nicolo-ribaudo , I see there's an issue for this here: babel/babel-eslint#679

@wyuenho
Copy link

wyuenho commented Sep 9, 2018

The doc is not fixed. It still says decoratorsBeforeExport defaults to false.

screen shot 2018-09-09 at 12 59 09

@wyuenho
Copy link

wyuenho commented Sep 9, 2018

@nicolo-ribaudo @loganfsmyth @existentialism The intention of #8465 is unclear, probably should write the tests a bit better so this is caught.

@xtuc
Copy link
Member

xtuc commented Sep 10, 2018

@wyuenho could you please open an issue on https://github.com/babel/website?

@jcabrerazuniga
Copy link

Sorry for the confusion, I get that old/new proposal doesn't mean anything for many people.

The "old" proposal is the one y'all are used to: it's the only one implemented in Babel and TypeScript. It's the one supported by babel-plugin-transform-decorators-legacy and @babel/plugin-transform-decorators, { legacy: true }. In @babel/parser it is decorators-legacy.

The "new" proposal is currently only implemented in @babel/parser (using the decorators plugin) and in babel-eslint, because it uses @babel/parser. We are implementing it (#7976), and also the TypeScript team is.

The decoratorsBeforeExport option is only meaningful for the "new" version of the proposal because, as you noted, there isn't consensus yet. You can't really use it in @babel/plugin-proposal-decorators because #7976 has not been merged yet, but you can use it in the @babel/parser's decorators plugin.

with this plugin (@babel/plugin-proposal-decorators) can you do decorators like export default class Thing at all? you could in the beta version, was this removed?

You still can, the old behavior hasn't changed. We only require the legacy option now:

const out = babel.transformSync(
  `
  @decorator
  export default class Foo {}
  `,
  {
    plugins: [
      [
        require("./packages/babel-plugin-proposal-decorators"),
        { legacy: true },
      ],
    ],
  }
);

console.log(out.code);

logs

var _class;

let Foo = decorator(_class = class Foo {}) || _class;

export { Foo as default };

could this simply be a problem with babel-eslint @9 ? where it is indeed parsing, but somehow the linter is throwing anyways? (maybe there is non parity between the linter and parser?)

In babel-eslint you have to opt-in to use the "old" proposal: in your eslint config, you should have something like this:

{
  parserOptions: {
    ecmaFeatures: {
      legacyDecorators: true
    }
  }
}

How can you set the legacyDcorators:false in webpack?

@nicolo-ribaudo
Copy link
Member

What is your current config?

@charlesritchea
Copy link

Yes, please explain how to resolve this with a webpack config. I dont' even know where to start with this error.

@nicolo-ribaudo
Copy link
Member

What is your current config?

@charlesritchea
Copy link

@nicolo-ribaudo This is my webpack 3.x config, which is used by electron-webpack. I'm taking over this project and I'm very ignorant of why things are configured the way they are.

.babelrc

{
  "presets": [
    "babel-preset-es2015-allow-top-level-this"
  ],
  "plugins": [
    "add-module-exports",
    "transform-remove-strict-mode"
  ]
}

webpack.config.js

// This configuration will be automatically merged with the electron-build default webpack configuration
// https://webpack.electron.build/modifying-webpack-configurations
const projectConfig = require('./project.config');
const webpack = require('webpack');
const isDev = process.env.NODE_ENV === "development";

module.exports = {
    module: {
        rules: [
            {
                test: /\.js$/,
                use: {
                    loader: 'babel-loader'
                }
            },
            {
                test: /\.jsx$/,
                use: {
                    loader: 'babel-loader',
                    options: {
                        presets: ['react']
                    }
                }
            },
            {
                test: /\.css$/,
                use: [{
                    loader: "style-loader"
                }, {
                    loader: "css-loader"
                }]
            },
            {
                test: /\.less$/,
                use: [{
                    loader: "style-loader"
                }, {
                    loader: "css-loader"
                }, {
                    loader: "postcss-loader"
                }, {
                    loader: "less-loader"
                }]
            },
            {
                test: /\.WOFF$/,
                use: "url-loader?mimetype=application/font-woff"
            },
            {
                test: /\.json$/,
                use: "json-loader"
            },
            {
                test: /\.txt$/,
                use: 'raw-loader'
            },
            {
                test: /\.waveform/,
                use: 'arraybuffer-loader'
            }
        ]
    },
    devtool: isDev ? 'eval-source-map' : 'source-map',
    plugins: [
        new webpack.DefinePlugin({
            'process.env.NODE_ENV': isDev ? JSON.stringify('development') : JSON.stringify('production')
        }),
        new webpack.ProvidePlugin({
            _: 'underscore',
            $: 'jquery',
            jQuery: 'jquery',
            'window.jQuery': 'jquery',
            Backbone: 'backbone',
            Bb: 'backbone',
            Marionette: 'backbone.marionette',
            Mn: 'backbone.marionette',
            d3: 'd3'
        })
    ],
    resolve: {
        modules: [
            projectConfig.srcDirectoryPath,
            projectConfig.nodeModulesDirectoryPath
        ],
        alias: {
            root: projectConfig.rootDirectoryPath,
            apps: projectConfig.appsDirectoryPath,
            data: projectConfig.dataDirectoryPath,
            svg: projectConfig.svgDirectoryPath,
            modules: projectConfig.nodeModulesDirectoryPath,
            'jquery-ui': projectConfig.jQueryUIDirectoryPath
        }
    }
};

And of course when attempting to upgrade to Webpack 4.x, I've changed the babel 6 versions of things to the babel 7 @babel\preset-react etc. I've also told electron-webpack to not even use this config, just the default, and I still get the same decoratorsBeforeExport error. I'm not even using decorators.

Unfortunately I don't have my Webpack 4.x attempt because it's just too frustrating of an endeavor right now, so I had to revert and move on.

@nicolo-ribaudo
Copy link
Member

You can run npx nls why @babel/plugin-proposa-decorators to see why it is installed.

@charlesritchea
Copy link

charlesritchea commented Nov 6, 2018 via email

@Aszmel
Copy link

Aszmel commented Jan 6, 2019

recently was getting problem with this error and installed
npm install --save-dev @babel/plugin-proposal-decorators
and setup
"babel": { "plugins": [ [ "@babel/plugin-proposal-decorators", { "legacy": true } ] ], "presets": [ "react-app" ] },

this run my code with some errors, but app was working, I'm a beginner, so hope this comment is relevant

@sergiotapia
Copy link

For would be Googler's and create-react-app users.

Create your create-react-app.

Then eject the project:

yarn eject

Add mobx, mobx-react and the decorator plugin:

yarn add @babel/plugin-proposal-decorators mobx mobx-react

Then in your package.json you'll see:

"babel": {
  "presets": [
    "react-app"
  ]
}

Just update it to:

"babel": {
  "plugins": [
    [
      "@babel/plugin-proposal-decorators",
      {
        "legacy": true
      }
    ]
  ],
  "presets": [
    "react-app"
  ]
}

@hoenth
Copy link

hoenth commented Apr 23, 2019

@sergiotapia Did you mean 'Then in your .babelrc`

@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jul 23, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jul 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
i: docs outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

15 participants