Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Decorators transpilation is broken in React preset #342

Closed
constgen opened this issue Oct 4, 2017 · 9 comments
Closed

Decorators transpilation is broken in React preset #342

constgen opened this issue Oct 4, 2017 · 9 comments

Comments

@constgen
Copy link
Contributor

constgen commented Oct 4, 2017

Have spent 2 days without result to integrate decorators transpilation to Babel on project with neutrino-preset-react. The issue is not reproduced on the same code base but on neutrino-preset-web. I tried all suggestions from Internet in all possible combinations but nothing worked. The error sounds like

Method has decorators, put the decorator plugin before the classes one

This is a test suite https://mega.nz/#!rN1WwYIK!hOBFn6E5akz522HOnlQFysoy8HKLEH-l4vRydOYqv7A . Switch preset in .neutrinorc.js and run again npm start to see different behavior.

Had no problems with the similar configuration in Neutrino 5. I believe the issue appeared starting from Neutrino 6 and later when we included class properties syntax by default to React preset.

It may be a serious blocker because decorators are necessary for mobx and react-dnd.

@constgen constgen changed the title Decprators transpilation is broken in React preset Decorators transpilation is broken in React preset Oct 4, 2017
@eliperelman
Copy link
Member

In Slack @timkelty posted this comment for how he was using decorators in his project:

module.exports = neutrino => {
  const applyDecorators = plugins => {
      plugins = [
        'babel-plugin-transform-decorators-legacy',
        'babel-plugin-transform-decorators',
        'babel-plugin-transform-class-properties',
        ...plugins
      ];

      return plugins;
  };

  neutrino.use(require.resolve('neutrino-preset-react'));

  neutrino.config.module
  .rule('compile')
  .use('babel')
  .tap(options => {
    options.plugins = applyDecorators(options.plugins);
    options.env.development.plugins = applyDecorators(options.env.development.plugins);

    return options;
  });
}

Hope that helps.

@timkelty
Copy link
Contributor

timkelty commented Oct 4, 2017

@constgen That got me working with mobx, specifically.

@constgen
Copy link
Contributor Author

constgen commented Oct 5, 2017

Thanks a lot. This information was helpful. Actually more short variant worked for me

const decoratorsPlugin = require.resolve('babel-plugin-transform-decorators-legacy');
const classPropertiesPlugin = require.resolve('babel-plugin-transform-class-properties');

config.module.rule('compile').use('babel').tap((options) => {
    options.plugins.unshift(decoratorsPlugin, classPropertiesPlugin);
    options.env.development.plugins.unshift(decoratorsPlugin, classPropertiesPlugin);

    return options;
});

My concerns:

  • babel-plugin-transform-class-properties is already included in neutrino-preset-react. Why should we include it twice?
  • What happens in Babel when we include the same plugin (probably with different versions) several times?

@timkelty
Copy link
Contributor

timkelty commented Oct 5, 2017

@constgen yep, your example does seem like it should be sufficient. Not sure why I had babel-plugin-transform-class-properties again…

It does seem like a particularly tricky way to have to go about this. What would be right way to address this in a new version of neutrino? Should neutrino-preset-react have a decorators option?

@eliperelman
Copy link
Member

Another variant could be:

const { merge } = require('neutrino-middleware-compile-loader');

const decorators = require.resolve('babel-plugin-transform-decorators-legacy');
const classes = require.resolve('babel-plugin-transform-class-properties');

config.module.rule('compile').use('babel').tap(options => merge({
  plugins: [decorators, classes],
  env: {
    development: {
      plugins: [decorators, classes]
    }
  }
}, options);

I don't think adding a decorators option in the preset is the right thing because:

  • Decorators still do not have a consistent or standard syntax
  • Where do we draw the line on babel plugins/presets options?

Trying to document how to easily merge babel configurations would be useful though, through the use of the compile loader's merge function.

@constgen
Copy link
Contributor Author

constgen commented Oct 5, 2017

Should neutrino-preset-react have a decorators option?

I think we should discuss this as one of the options. Another one is to clearly document this as we do with Hot Module Replacement.

@eliperelman
Copy link
Member

I think we should discuss this as one of the options.

As mentioned, I don't think adding a decorators option in the preset is the right thing because:

  • Decorators still do not have a consistent or standard syntax
  • Where do we draw the line on babel plugins/presets options?

Another one is to clearly document this as we do with Hot Module Replacement.

Trying to document how to easily merge babel configurations would be useful though, through the use of the compile loader's merge function.

@vladfr
Copy link

vladfr commented Apr 27, 2018

This is not working for me, nor the solutions provided.

I've changed two things:

  • I'm starting from neutrino-preset-web
  • I reversed the order of the merged plugins, I've put class-properties before decorators. Couldn't get @observable and @action to work properly without this, or with decorators first.

Here's my full .neutrinorc.js:

const compileLoader = require('@neutrinojs/compile-loader');

module.exports = {
  use: [
    ['@neutrinojs/react', {
        hot: true,
        style: {
          style: {},
          css: {},
          loaders: [
            {
              loader: 'sass-loader',
              test: /\.scss$/,
              useId: 'sass',
              options: {
                includePaths: ['node_modules', 'src/scss', '**/scss']
              }
            }
          ],
          test: /\.s?css$/,
          ruleId: 'style',
          styleUseId: 'style',
          cssUseId: 'css',
          hotUseId: 'hot',
          hot: true,
          modules: true,
          modulesSuffix: '-modules',
          modulesTest: /\.module.css$/,
          extractId: 'extract',
          extract: {
            plugin: {},
            loader: {}
          }
        },
        output:{
          publicPath: '/public/'
        }
      },
    ],
    neutrino => {
      neutrino.config.module
      .rule('compile')
        .use('babel')
          .tap(options => 
            compileLoader.merge({
              plugins: [
                require.resolve('babel-plugin-transform-class-properties'),
                require.resolve('babel-plugin-transform-decorators-legacy')
              ]
            }, options)
          )
        },
    ['@neutrinojs/image-loader', {
      limit: 8192,
      svg: {},
      img: {},
      ico: {}
    }],
    '@neutrinojs/mocha',
    'neutrino-preset-postcss-autoprefixer'
  ]
};

cc @constgen @eliperelman

@constgen
Copy link
Contributor Author

constgen commented May 3, 2018

Recently I have reviewed changes in @neutrinojs/react and I am not completely sure that the solution above may work on the latest Neutrino version. @vladfr, can you show the console output with errors?

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

No branches or pull requests

4 participants