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
[Doc PR] naming fix in example #5659
Conversation
because code is declared as const twice
@aretecode, thanks for your PR! By analyzing the history of the files in this pull request, we identified @xtuc, @existentialism and @novemberborn to be potential reviewers. |
align the naming, allow the example to be usable without change
Codecov Report
@@ Coverage Diff @@
## 7.0 #5659 +/- ##
=======================================
Coverage 84.38% 84.38%
=======================================
Files 284 284
Lines 9760 9760
Branches 2735 2735
=======================================
Hits 8236 8236
Misses 1010 1010
Partials 514 514
Continue to review full report at Codecov.
|
packages/babel-core/README.md
Outdated
const ast = babylon.parse(code, { allowReturnOutsideFunction: true }); | ||
const { code, map, ast } = babel.transformFromAst(ast, code, options); | ||
const string = "if (true) return;"; | ||
const parsedAst = babel.parse(string, { allowReturnOutsideFunction: true }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be kept babylon.parse()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@existentialism updated
packages/babel-core/README.md
Outdated
const code = "if (true) return;"; | ||
const ast = babylon.parse(code, { allowReturnOutsideFunction: true }); | ||
const { code, map, ast } = babel.transformFromAst(ast, code, options); | ||
const string = "if (true) return;"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could call this sourceCode
, what do you think?
packages/babel-core/README.md
Outdated
const { code, map, ast } = babel.transformFromAst(ast, code, options); | ||
const string = "if (true) return;"; | ||
const parsedAst = babylon.parse(string, { allowReturnOutsideFunction: true }); | ||
const { code, map, ast } = babel.transformFromAst(parsedAst, string, options); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be called generatedCode
Good catch and thanks for your PR. I added a few notes, could you tell me what do you think about it. The idea is too make the example as clear as possible. |
@xtuc variable names that describe their intention is 👍 👌 |
packages/babel-core/README.md
Outdated
const ast = babylon.parse(code, { allowReturnOutsideFunction: true }); | ||
const { code, map, ast } = babel.transformFromAst(ast, code, options); | ||
const sourceCode = "if (true) return;"; | ||
const generatedCode = babylon.parse(sourceCode, { allowReturnOutsideFunction: true }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, it's the ast
, this was good. I meant the code variable above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xtina-starr that's the returned object property, can't be changed unless destructuring is removed. Main thing for this PR was just fixing the duplicated unclear variable name (now named as you suggested, sourceCode
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes sorry, that's correct (I don't believe I missed that)
@aretecode thanks! |
because code is declared as const twice