facebook / relay

Relay is a JavaScript framework for building data-driven React applications.
https://relay.dev
MIT License
18.38k stars 1.82k forks source link

Change in behavior between relay 1.6.0 to 1.6.2 Using the export keyword between a decorator and a class is not allowed #2505

Open soneymathew opened 6 years ago

soneymathew commented 6 years ago

I use the injectIntl decorator from react-intl in my project. While upgrading to 1.6.2 I noticed a change in behavior of relay-compiler.

I have made the changes in this fork to reproduce the issue. https://github.com/soneymathew/relay-examples/blob/b105481f3d4fd666eaaf39ed488b753c77bbace7/todo/js/components/TodoApp.js#L32

$ npm run build

> @ build /Users/me/Documents/relay-examples/todo
> relay-compiler --src ./js/ --schema ./data/schema.graphql

HINT: pass --watch to keep watching for changes.
ERROR:
Parse error: SyntaxError: Using the export keyword between a decorator and a class is not allowed. Please use `export @dec class` instead. (33:0) in "components/TodoApp.js"
npm ERR! code ELIFECYCLE
npm ERR! errno 100
npm ERR! @ build: `relay-compiler --src ./js/ --schema ./data/schema.graphql`
npm ERR! Exit status 100
npm ERR!
npm ERR! Failed at the @ build script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/me/.npm/_logs/2018-08-11T00_09_09_927Z-debug.log
soneymathew commented 6 years ago

In case any one is running into this, here is a rule to help you find the occurrences, if you use eslint https://astexplorer.net/#/gist/aba9ffaf77d163b5a9b7318e0d3ee455/5200fff2efa44a6e1343b8038628981773b8d7e1

export default function(context) {
  function isClassDeclaration(node) {
    return node.declaration && 
         node.declaration.type==="ClassDeclaration" && 
         node.declaration.decorators && node.declaration.decorators.length > 0;
  }
  return {
    ExportNamedDeclaration: function(node) {
      if(isClassDeclaration(node)) // handle only 
      context.report(node, 'Using the "export" keyword between a decorator and a class is not allowed. \nYou have ' + node.declaration.decorators.length + ' decorator(s)');
    },
    ExportDefaultDeclaration: function(node) {
      if(isClassDeclaration(node)) // handle only 
      context.report(node, 'Using the "export default" keyword between a decorator and a class is not allowed. \nYou have ' + node.declaration.decorators.length + ' decorator(s)');
    }
  };
};
sibelius commented 6 years ago

This is a babel 7 issue

piotrblasiak commented 6 years ago

Is there any workaround for this? Annoyingly Prettier also formats the export & decorators in the "wrong" order...

soneymathew commented 6 years ago

@piotrblasiak I worked around this by separating the export as a separate line

@decorator1
class MyClass extends Component<Props> {
.... 
.... 
}
export default MyClass;

for a larger project you may have to use something like jscodeshift code mod, apparently it's not easy to use a eslint --fix rule strategy for this scenario, way too complicated when I tried.

I think a good way to dig yourself out of this is to

  1. write a code mod (do it by hand if project is small enough)
  2. enforce by eslint
  3. remove the legacy decorator transform
kassens commented 6 years ago

We started using @babel/parse (babel 7 replacement of babylon) to parse files with other new syntax features. Did someone already report an issue with Babel? I assume one of the next parser updates should fix the issue again.

soneymathew commented 6 years ago

@kassens is there a chance that relay compiler can use the .babelrc from userland? I think @sibelius mentioned this a while back.

kassens commented 6 years ago

Maybe? My knowledge in how to correctly load the babel config with all the various ways to define the transforms and versions is limited.

soneymathew commented 6 years ago

yeah this can backfire as well I guess, could maybe a point of consideration in this RFC https://github.com/facebook/relay/issues/2518

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.