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

Function argument destructuring and default value generates "syntax" error #201

Closed
3 of 10 tasks
alekbarszczewski opened this issue Mar 23, 2018 · 10 comments
Closed
3 of 10 tasks

Comments

@alekbarszczewski
Copy link

This is a:

  • Bug Report
  • Feature Request
  • Question
  • Other

Which concerns:

  • flow-runtime
  • babel-plugin-flow-runtime
  • flow-runtime-validators
  • flow-runtime-mobx
  • flow-config-parser
  • The documentation website

What is the current behaviour?

const defaultTrx = 'abc'
class Test {
  static async getLastUpdateTimestamp ({ trx }: { trx: any } = { trx: defaultTrx }) { /* do nothing */ }
}
  • flow check itself (flow check --strip-root) does not complain about this
  • this function works as expected on runtime

However when I enable babel-plugin-flow-runtime and then I start my program I get following error:

Duplicate declaration "trx"

.babelrc.js:

module.exports = {
  presets: [
    '@babel/flow',
    [ '@babel/env', {
      targets: {
        node: 'current'
      }
    }]
  ],
  plugins: [
    '@babel/proposal-class-properties',
    '@babel/proposal-object-rest-spread',
    '@babel/proposal-export-namespace',
    '@babel/proposal-decorators',
    ['root-import', {
      rootPathPrefix: '@',
      rootPathSuffix: 'src'
    }],
    ['flow-runtime', {
      assert: false,
      annotate: false
    }]
  ],
  env: {
    test: {
      plugins: ['istanbul']
    }
  }
}

Without flow-runtime plugin in .babelrc.js it works during runtime (no syntax errors) and also flow does not complain about it during flow check.


What is the expected behaviour?

It should not throw error, this is correct syntax and it's supported by flow.


Which package versions are you using?

"flow-runtime": "^0.17.0",
"babel-plugin-flow-runtime": "^0.17.0",
"@babel/preset-flow": "^7.0.0-beta.32"
@linonetwo
Copy link

linonetwo commented May 23, 2018

Same on me:

Module build failed: TypeError: /Users/.../src/components/Formula/index.js: Duplicate declaration "period"
  31 |     }
  32 |   }
> 33 |   focusVariable = ({ period, field, position }: { period: string, field: string, position: string }) =>
     |                      ^
  34 |     this.setState({ period, field, position });
  35 |   render() {
  36 |     const equalityOperatorIndex = 1;

And if remove babel-plugin-flow-runtime it works fine.

Some of my deps:

    "babel-plugin-flow-runtime": "^0.17.0",
    "react-scripts": "^2.0.0-next.66cc7a90",

@linonetwo
Copy link

linonetwo commented Jun 26, 2018

It only happened when NODE_ENV=production.

@s-ol
Copy link

s-ol commented Sep 19, 2018

I'm having the same issue:

/mnt/space/notification_worker/node_modules/@babel/core/node_modules/@babel/traverse/lib/scope/index.js:347
      throw this.hub.buildError(id, `Duplicate declaration "${name}"`, TypeError);
      ^

TypeError: /mnt/space/notification_worker/index.handler.sms.js: Duplicate declaration "timestamp"
  107 |                 })
  108 |               */
> 109 |                 .then(({ timestamp, data }: Notification) => {
      |                          ^
  110 |                   console.log(`${timestamp}: handling ${msg.fields.routingKey} event...`);
  111 |                   return data;
  112 |                 })
    at File.buildCodeFrameError (/mnt/space/notification_worker/node_modules/@babel/core/lib/transformation/file/file.js:261:12)
    at Scope.checkBlockScopedCollisions (/mnt/space/notification_worker/node_modules/@babel/core/node_modules/@babel/traverse/lib/scope/index.js:347:22)
    at Scope.registerBinding (/mnt/space/notification_worker/node_modules/@babel/core/node_modules/@babel/traverse/lib/scope/index.js:504:16)
    at Scope.registerDeclaration (/mnt/space/notification_worker/node_modules/@babel/core/node_modules/@babel/traverse/lib/scope/index.js:444:14)
    at Object.BlockScoped (/mnt/space/notification_worker/node_modules/@babel/core/node_modules/@babel/traverse/lib/scope/index.js:189:28)
    at Object.newFn (/mnt/space/notification_worker/node_modules/@babel/core/node_modules/@babel/traverse/lib/visitors.js:230:17)
    at NodePath._call (/mnt/space/notification_worker/node_modules/@babel/core/node_modules/@babel/traverse/lib/path/context.js:53:20)
    at NodePath.call (/mnt/space/notification_worker/node_modules/@babel/core/node_modules/@babel/traverse/lib/path/context.js:36:14)
    at NodePath.visit (/mnt/space/notification_worker/node_modules/@babel/core/node_modules/@babel/traverse/lib/path/context.js:88:12)
    at TraversalContext.visitQueue (/mnt/space/notification_worker/node_modules/@babel/core/node_modules/@babel/traverse/lib/context.js:118:16)

if I work around the destructuring I get another one in a much weirder place:

/mnt/space/notification_worker/node_modules/@babel/core/node_modules/@babel/traverse/lib/scope/index.js:347
      throw this.hub.buildError(id, `Duplicate declaration "${name}"`, TypeError);
      ^

TypeError: /mnt/space/notification_worker/api/collection.js: Duplicate declaration "cache" (This is an error on an internal node. Probably an internal error.)
    at File.buildCodeFrameError (/mnt/space/notification_worker/node_modules/@babel/core/lib/transformation/file/file.js:261:12)
    at Scope.checkBlockScopedCollisions (/mnt/space/notification_worker/node_modules/@babel/core/node_modules/@babel/traverse/lib/scope/index.js:347:22)
    at Scope.registerBinding (/mnt/space/notification_worker/node_modules/@babel/core/node_modules/@babel/traverse/lib/scope/index.js:504:16)
    at Scope.registerDeclaration (/mnt/space/notification_worker/node_modules/@babel/core/node_modules/@babel/traverse/lib/scope/index.js:444:14)
    at Object.BlockScoped (/mnt/space/notification_worker/node_modules/@babel/core/node_modules/@babel/traverse/lib/scope/index.js:189:28)
    at Object.newFn (/mnt/space/notification_worker/node_modules/@babel/core/node_modules/@babel/traverse/lib/visitors.js:230:17)
    at NodePath._call (/mnt/space/notification_worker/node_modules/@babel/core/node_modules/@babel/traverse/lib/path/context.js:53:20)
    at NodePath.call (/mnt/space/notification_worker/node_modules/@babel/core/node_modules/@babel/traverse/lib/path/context.js:36:14)
    at NodePath.visit (/mnt/space/notification_worker/node_modules/@babel/core/node_modules/@babel/traverse/lib/path/context.js:88:12)
    at TraversalContext.visitQueue (/mnt/space/notification_worker/node_modules/@babel/core/node_modules/@babel/traverse/lib/context.js:118:16)

the variable 'cache' is the one in this helper (verified by renaming, the error log changes with it):

  buildCache() {
    type Extended = Raw & Helpers;
    const cache: { [string]: Extended } = {};

    return (_id: string): Promise<Extended> => {
      if (cache[_id]) return Promise.resolve(cache[_id]);

      return this.findOne({ _id })
        .then(doc => {
          cache[_id] = doc;
          return doc;
        });
    };
  }

personally I don't see any commonality between these two cases but maybe it can help someone else figure the issue out as it seems to be related as per the error message.
Luckily in my project flow-runtime isn't used extensively so I will be able to remove it for the time being.

@golmansax
Copy link

golmansax commented Feb 8, 2019

Any updates on this? I'm still getting the destructuring error with babel-plugin-flow-runtime@0.18.0.

    ERROR in ./app/frontend/addresses/decorator.js
    Module build failed (from ./node_modules/babel-loader/lib/index.js):
    TypeError: ./app/frontend/addresses/decorator.js: Duplicate declaration "pinColor"
      74 |   }
      75 |
    > 76 |   getMapboxImageSrc({ pinColor }: MapboxImageOptions) {

@jedwards1211
Copy link
Collaborator

jedwards1211 commented May 1, 2019

I just ran into this myself, I will try to debug it.
Surprisingly this error happens even with {assert: false, annotate: false} options.

@jedwards1211
Copy link
Collaborator

@jedwards1211
Copy link
Collaborator

Alright, the problem is that babel has already registered bindings for the function parameters before preTransformVisitors folds them into the body. So preTransformVisitors needs to remove those bindings.

@jedwards1211
Copy link
Collaborator

jedwards1211 commented May 1, 2019

@phpnode I just discovered that the existing test setup can't catch this, because it calls traverse repeatedly on the root node, creating new NodePaths/scopes on each traverse. We need to make the test transform work more like the actual babel plugin.

@jedwards1211
Copy link
Collaborator

fix released in v0.19.0 of babel-plugin-flow-runtime!

@linonetwo
Copy link

My God! Finally, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants