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

Feature request: support Babel 7 #184

Closed
3 tasks done
OKNoah opened this issue Dec 31, 2017 · 13 comments
Closed
3 tasks done

Feature request: support Babel 7 #184

OKNoah opened this issue Dec 31, 2017 · 13 comments

Comments

@OKNoah
Copy link

OKNoah commented Dec 31, 2017

This is a:

  • Bug Report
  • Feature Request

Which concerns:

  • babel-plugin-flow-runtime

What is the current behaviour?

The complete configuration is here: https://github.com/OKNoah/flow-runtime-test/pull/2/files

{
  "presets": [
    "@babel/preset-env",
    "@babel/preset-stage-2"
  ],
  "plugins": [
    ["flow-runtime"],
    "transform-decorators-legacy"
  ]
}
/Users/noah/Projects/flow-runtime-test/node_modules/@babel/core/lib/transformation/normalize-file.js:136
    throw err;
    ^

SyntaxError: /Users/noah/Projects/flow-runtime-test/index.js: Unexpected token, expected ";" (3:5)

  1 | // @flow-runtime
  2 | 
> 3 | type User = {
    |      ^
  4 |   name: string
  5 | };
  6 | 
    at Parser.raise (/Users/noah/Projects/flow-runtime-test/node_modules/@babel/core/node_modules/babylon/lib/index.js:832:15)
    at Parser.unexpected (/Users/noah/Projects/flow-runtime-test/node_modules/@babel/core/node_modules/babylon/lib/index.js:2212:16)
    at Parser.semicolon (/Users/noah/Projects/flow-runtime-test/node_modules/@babel/core/node_modules/babylon/lib/index.js:2192:40)
    at Parser.parseExpressionStatement (/Users/noah/Projects/flow-runtime-test/node_modules/@babel/core/node_modules/babylon/lib/index.js:4729:10)
    at Parser.parseStatementContent (/Users/noah/Projects/flow-runtime-test/node_modules/@babel/core/node_modules/babylon/lib/index.js:4327:19)
    at Parser.parseStatement (/Users/noah/Projects/flow-runtime-test/node_modules/@babel/core/node_modules/babylon/lib/index.js:4206:17)
    at Parser.parseBlockOrModuleBlockBody (/Users/noah/Projects/flow-runtime-test/node_modules/@babel/core/node_modules/babylon/lib/index.js:4764:23)
    at Parser.parseBlockBody (/Users/noah/Projects/flow-runtime-test/node_modules/@babel/core/node_modules/babylon/lib/index.js:4750:10)
    at Parser.parseTopLevel (/Users/noah/Projects/flow-runtime-test/node_modules/@babel/core/node_modules/babylon/lib/index.js:4174:10)
    at Parser.parse (/Users/noah/Projects/flow-runtime-test/node_modules/@babel/core/node_modules/babylon/lib/index.js:5613:17)
[nodemon] app crashed - waiting for file changes before starting...

What is the expected behaviour?

Should run without errors and do a console log.


Which package versions are you using?

"@babel/core": "^7.0.0-beta.36",
"@babel/preset-env": "^7.0.0-beta.36",
"@babel/preset-stage-2": "^7.0.0-beta.36",
"@babel/register": "^7.0.0-beta.36",
"babel-plugin-flow-runtime": "^0.15.0",
"babel-plugin-transform-decorators-legacy": "^1.3.4",
"flow-runtime": "^0.16.0"
@jedwards1211 jedwards1211 changed the title Babel 7 Feature request: support Babel 7 Jan 22, 2018
@jedwards1211
Copy link
Collaborator

I don't know about anyone else, but I probably won't take a stab at Babel 7 support until it's out of beta.

@phpnode
Copy link
Collaborator

phpnode commented Feb 7, 2018

agree with @jedwards1211 - we obviously want to support 7 but I'm wary of being an early adopter

@JacopKane
Copy link

Currently, this is the only one preset that is stopping us migrating to babel 7.
Wish I've had some AST experience. I will try to check it out later if it's something I can handle

@billinghamj
Copy link

They have stated that it is effectively ready for production use and no significant breaking changes are to be expected.

@gajus
Copy link
Owner

gajus commented Jun 1, 2018

@phpnode Where do I donate to get this done? :-)

Seriously though... I think this project is of incredibly high value to a lot of us. You should consider accepting contributions in a monetary form, esp. now that there are so many ways of accepting donations.

@devcorpio
Copy link

devcorpio commented Jun 7, 2018

Same problem here :(, we are trying to update from nextjs 5.1 to nextjs 6.0.3, but we are blocked too:

`> Module build failed: SyntaxError: /components/Card/Card.js: Unexpected token, expected ";" (10:5)

8 | import styles from './cardStyles';
9 |

10 | type Props = {
| ^
11 | title: string,
`

@jedwards1211
Copy link
Collaborator

@devcorpio nextjs takes that much control of the build pipeline that there's no way to override what it's using to transpile JS files?

@rej156
Copy link

rej156 commented Jul 4, 2018

Bump

@vjpr
Copy link

vjpr commented Jul 9, 2018

Started my own babel-7 branch here: https://github.com/vjpr/flow-runtime/tree/vjpr/babel-7

So far just replaced all deps and usages with babel@7 equivalents.

Will be following the API migration guide: https://babeljs.io/docs/en/next/v7-migration-api

I hope only the babel-plugin needs replacing. Should be okay to leave babel@6 as the toolchain for building flow-runtime itself.


After making these minor changes, I seem to need babel-plugin-transform-flow-strip-types or it throws syntax errors on the flow syntax.


Currently stuck on:

Error: TypeError: test.js: Duplicate declaration "bar" (This is an error on an internal node. Probably an internal error.)

For this code sample:

export function foo() {
  {
    const bar: number = 1
  }
}

@vjpr
Copy link

vjpr commented Jul 10, 2018

I traced down the exact code that is causing this snippet to fail.

export function foo() {
  {
    const bar: number = 1
  }
}
Error: TypeError: test.js: Duplicate declaration "bar" (This is an error on an internal node. Probably an internal error.)

https://github.com/codemix/flow-runtime/blob/8068da7c73225aad569fb46b6d981ddd6461ff4a/packages/babel-plugin-flow-runtime/src/transformVisitors.js#L276-L286

If a change t.identifier(name) to t.identifier(name + 'blah') the error is gone and the generated code looks fine. This makes me think it is related to how variables are tracked within scopes.

@jedwards1211 Could you shed some light on how this works?


Adding path.scope.removeOwnBinding(name) above context.replacePath fixed this case for me...but I'm very patchy on how this is actually working.

Here we are replacing bar: number = 1 with bar = t.warn(t.number(), 1). When running DEBUG=babel* it looks like there are multiple traversals through the code which do different things. Each traversal will collect up the bindings as it traverses down the tree and encounters scopes. Part of this process involves checking for scope collisions/duplicates which is where our error comes from.

After we have run our replace, there seem to be multiple bindings hanging around. There is some discussion about something similar here: airbnb/babel-plugin-inline-react-svg#31 (comment)

Seeing as it only occurs if we have a named export and an ES6 block statement, perhaps it is more of a babel@7 bug.


Published as @vjpr/[email protected] and @vjpr/[email protected]. Works for me, but tests are not passing and can't guarantee things like work.

@0rvar
Copy link
Contributor

0rvar commented Aug 28, 2018

Babel 7 has been released!

Repository owner deleted a comment from bsmith-cycorp Sep 14, 2018
@semireg
Copy link

semireg commented Nov 1, 2018

@vjpr

Seeing as it only occurs if we have a named export and an ES6 block statement, perhaps it is more of a babel@7 bug.

Here are the pertinent babel@7 issues currently open: babel/babel#8899, babel/babel#8525, babel/babel#8559

I haven't dug in too deep, but after fixing #216 I'm now seeing "Duplicate declarations" when I use the same bundling command.

The dupes I've seen so far seem to generate from destructuring parameters:

TypeError: /path/node_modules/react-native/Libraries/Interaction/TaskQueue.js: Duplicate declaration "onMoreTasks"
  50 |    * tasks to process.
  51 |    */
> 52 |   constructor({onMoreTasks}: {onMoreTasks: () => void}) {
     |                ^
  53 |     this._onMoreTasks = onMoreTasks;
  54 |     this._queueStack = [{tasks: [], popable: false}];
  55 |   }
    at File.buildCodeFrameError (/path/node_modules/@babel/core/lib/transformation/file/file.js:261:12)
    at Scope.checkBlockScopedCollisions (/path/node_modules/@babel/traverse/lib/scope/index.js:347:22)
    at Scope.registerBinding (/path/node_modules/@babel/traverse/lib/scope/index.js:504:16)
    at Scope.registerDeclaration (/path/node_modules/@babel/traverse/lib/scope/index.js:444:14)
    at Object.BlockScoped (/path/node_modules/@babel/traverse/lib/scope/index.js:189:28)
    at Object.newFn (/path/node_modules/@babel/traverse/lib/visitors.js:230:17)
    at NodePath._call (/path/node_modules/@babel/traverse/lib/path/context.js:53:20)
    at NodePath.call (/path/node_modules/@babel/traverse/lib/path/context.js:36:14)
    at NodePath.visit (/path/node_modules/@babel/traverse/lib/path/context.js:88:12)

Yet, I can't reproduce the failure in the tests. This babel-plugin-flow-runtime test passes:

export const input = `
function foo({bar}: {bar:string}) {
  console.log(bar);
}
`;

export const expected = `
import t from "flow-runtime";
function foo(_arg) {
  let {
    bar
  } = t.object(t.property("bar", t.string())).assert(_arg);
  console.log(bar);
}
`;

This flow-runtime test also passes. I'm less sure if this fully leverages the fixture, though.

/* @flow */

export function foo({bar}: {bar:string}): string {
  return bar;
}

export function pass(t: TypeContext) {
  foo('test');
  return true;
}

export function fail(t: TypeContext) {
  const foo: string = foo(1);
  return false;
}

@jedwards1211
Copy link
Collaborator

Fixed in v0.18.0!

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

No branches or pull requests