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

Add support for Babel 7 #996

Closed
kripod opened this issue Jan 17, 2018 · 18 comments
Closed

Add support for Babel 7 #996

kripod opened this issue Jan 17, 2018 · 18 comments

Comments

@kripod
Copy link
Contributor

kripod commented Jan 17, 2018

As of today, having a .babelrc file with the preset @babel/preset-env makes the documentation build command throw an error.

@tmcw
Copy link
Member

tmcw commented Jan 17, 2018

Customarily I wait until projects are in stable release state to update to them (babel 7 is still a beta). That said, if anyone wants to help out and write a PR for this issue (or any other!) happy to review & merge.

@anthony-redFox
Copy link
Member

I have tried to migrate on new babel version, but in babelify source hardcoded uses babel 6 version. it is blocking for us. waiting PR babel/babelify#255

@vihanb
Copy link

vihanb commented Feb 11, 2018

@anthony-redFox git+https://github.com/ylemkimon/babelify.git#master could be used instead of NPM package name in package.json until babelify is updated?

@ganapativs
Copy link

@tmcw
Copy link
Member

tmcw commented Aug 21, 2018

That's great news! Setting a reminder to check back on Friday and start a migration if it's landed.

@ghost
Copy link

ghost commented Sep 19, 2018

Any update on this?

@vihanb
Copy link

vihanb commented Sep 20, 2018

@suparng-medallia I believe #1114 has some more info on progress related to the upgrade

@tmcw
Copy link
Member

tmcw commented Sep 20, 2018

Sorry for the delay on this issue, folks - it's in my TODO to review, merge, and deploy documentation.js with babel 7 support this afternoon (PST), assuming everything goes according to plan.

@tmcw
Copy link
Member

tmcw commented Sep 21, 2018

All right - documentation.js 9.0.0-alpha.0 is published to npm. Please try it out, and report back on any failures or successes! It uses Babel 7 across the board, thanks to hard work from @vicapow and folks!

@haggholm
Copy link

I’m having issues with the alpha due to Babel configuration—we use legacy decorators and the syntax

@foo
export default class Cls { /* ... */ }

This is not supported by the default Babel configuration, which demands that the export come before the decorators:

export default @foo class Cls { /* ... */ }

Thus, running documentation on our code causes a Babel error:

 Using the export keyword between a decorator and a class is not allowed. Please use `export @dec class` instead. (47:0)
{ SyntaxError: Using the export keyword between a decorator and a class is not allowed. Please use `export @dec class` instead. (47:0)
    at _class.raise (/shared/owl-devtools/node_modules/@babel/parser/lib/index.js:3939:15)
    at _class.parseDecorators (/shared/owl-devtools/node_modules/@babel/parser/lib/index.js:7304:14)
    at _class.parseStatement (/shared/owl-devtools/node_modules/@babel/parser/lib/index.js:7142:12)
    at _class.parseStatement (/shared/owl-devtools/node_modules/@babel/parser/lib/index.js:1902:57)
    at _class.parseBlockOrModuleBlockBody (/shared/owl-devtools/node_modules/@babel/parser/lib/index.js:7696:23)
    at _class.parseBlockBody (/shared/owl-devtools/node_modules/@babel/parser/lib/index.js:7683:10)
    at _class.parseTopLevel (/shared/owl-devtools/node_modules/@babel/parser/lib/index.js:7110:10)
    at _class.parse (/shared/owl-devtools/node_modules/@babel/parser/lib/index.js:8510:17)
    at Object.parse (/shared/owl-devtools/node_modules/@babel/parser/lib/index.js:10462:38)
    at parseToAst (/shared/owl-devtools/node_modules/documentation/lib/parsers/parse_to_ast.js:32:22)
    at parseJavaScript (/shared/owl-devtools/node_modules/documentation/lib/parsers/javascript.js:51:44)
    at /shared/owl-devtools/node_modules/documentation/lib/index.js:131:12
    at arrayMap (/shared/owl-devtools/node_modules/documentation/node_modules/lodash/lodash.js:639:23)
    at map (/shared/owl-devtools/node_modules/documentation/node_modules/lodash/lodash.js:9556:14)
    at Function.flatMap (/shared/owl-devtools/node_modules/documentation/node_modules/lodash/lodash.js:9259:26)
    at buildInternal (/shared/owl-devtools/node_modules/documentation/lib/index.js:118:31) pos: 1884, loc: Position { line: 47, column: 0 } }

It does not seem to be possible to pass any configuration to Babel via documentation (correct me if I’m wrong). I’m not an expert on Babel, but I presume that export/decorator order is not the only thing that might conceivably fail in this way, though in this case it seems to be an explicit setting:

['decorators', { decoratorsBeforeExport: false }],

Might it be possible to read Babel config from the target directory, or at least pass customised options via documentation?

@tmcw
Copy link
Member

tmcw commented Sep 26, 2018

Was that working in documentation.js before the v7 support? I'm a little hesitant to add another option to the parser, this project is already awash in options to handle different edge cases.

@haggholm
Copy link

I’m not too sure; a variety of things weren’t working before…

While I take your point about edge cases, I wonder if this should really be considered as one. The ideal, IMO, would be to use the configuration of the project you’re running against. I expect that this would be most useful to most users—surely accepting the same syntax at build time and doc generation time is what most users would want and expect? When would that not be the right choice?

@tmcw
Copy link
Member

tmcw commented Sep 26, 2018

Sure, well - mostly because documentation.js is able to parse a variety of syntax forms, but opening the door to every possible syntax plugin means that it will then see ASTs that aren't handled at all in the inference stage, and that'll be logged as a bug. Given an ever-expanding set of plugins, that would put documentation.js in the position of having to support every possible plugin, every plugin one-by-one, or setting some sort of limit.

Which - I'm totally down with, if there's the contributor energy to do it, but I'm not confident that a user who just needs to run the tool just once with plugin X will invest the time making it work, rather than dropping a bug report.

At least in the medium-term future, I'm really leaning toward building tools that work with certain standardized subsets of the bleeding edge, like 'only stage 4 proposals', for example. Which cuts out a lot of users who are really pushing the babel toolchain to its limits, but allows me as a maintainer to focus on building a great thing solving the problem, rather than handling syntax variants.

@haggholm
Copy link

I definitely don’t know the internals of Babel’s ASTs, so I may just be speaking out of ignorance. Do syntax plugins generate invalid ASTs (from the POV of code that isn’t aware of it)? I guess my implicit assumption was that if a Babel syntax plugin consumes novel syntax, it would either generate a standard AST, or a subtree that you could shrug at and say “Well, that’s not supported”. For example, I figured that the decorator syntax plugin would generate the same AST albeit based on slightly different inputs. I mean, I for one wouldn’t assume that the tool would necessarily document my use of decorators; I just didn’t expect it to puke a syntax error. But of course, (we) users can be a pain sometimes…

@haggholm
Copy link

Add an extra wrinkle: Our project uses https://prettier.io/, which I gather is fairly popular. I experimentally reversed the order of decorators and export, and found that Prettier corrects it back to the legacy format. Presumably, then, enforcing export-before-decorators will break any project using Prettier.

@adirzoari
Copy link

@haggholm I have the same issue
" Using the export keyword between a decorator and a class is not allowed. Please use export @dec class instead."
How did you solve it?

@haggholm
Copy link

I got it to work by patching documentation using patch-package in our npm scripts.
documentation+9.0.0-alpha.1.zip

@tmcw
Copy link
Member

tmcw commented Apr 18, 2019

All right - custom babel configurations are now supported in 10.x like

documentation build --babel=./babel.config.js

We may support auto-resolving babel configs in the future with a 11.x, PRs gladly accepted in that direction.

@tmcw tmcw closed this as completed Apr 18, 2019
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

7 participants