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 alias docs for @babel/types #13151

Merged
merged 6 commits into from Apr 30, 2021
Merged

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Apr 14, 2021

Q                       A
License MIT

This PR adds a new "Alias" section to the docs of @babel/types. It reflects my understanding of current aliases, feedbacks are welcome!

This PR includes commits from #13148, will rebase once that one is merged.

See here for rendered results.

@JLHwung JLHwung added the PR: Docs 📝 A type of pull request used for our changelog categories label Apr 14, 2021
Terminatorless:
"A cover of AST nodes whose semantic will change when a line terminator is inserted between the operator and the operand.",
UnaryLike: "A cover of UnaryExpression and SpreadElement.",
UserWhitespacable: "",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea what is our intention for UserWhitespacable, it covers

ObjectMethod
ObjectProperty
ObjectTypeInternalSlot
ObjectTypeCallProperty
ObjectTypeIndexer
ObjectTypeProperty
ObjectTypeSpreadProperty

Copy link
Member

@hzoo hzoo Apr 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

b1fe449 based on this, it's probably something that used to be used in generator, we can probably just remove it honestly. Sounds like it wasn't intended to be used as a visitor at least but to check something for printing. It would be really odd if someone used it, we don't anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I update description to "deprecated".

@babel-bot
Copy link
Collaborator

babel-bot commented Apr 14, 2021

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/45208/

@codesandbox-ci
Copy link

codesandbox-ci bot commented Apr 14, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit e8ebe57:

Sandbox Source
babel-repl-custom-plugin Configuration
babel-plugin-multi-config Configuration

"A cover of functions and [method](#method)s, the must have `body` and `params`. Note: `Function` is different to `FunctionParent`.",
FunctionParent:
"A cover of AST nodes that start an execution context with new [VariableEnvironment](https://tc39.es/ecma262/#table-additional-state-components-for-ecmascript-code-execution-contexts). In other words, they define the scope of `var` declarations. FunctionParent did not include `Program` since Babel 7.",
Immutable: "A cover of immutable AST nodes.",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we come up with a formal definition of Immutable? What is the difference between Pureish and Immutable?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the original intention was

  • Immutable -> You cannot define properties on it (+ JSX elements which behave as you can't otherwise you break JSX libraries)
  • Pureish -> It doesn't have side effects

We will probably need to audit all these aliases before Babel 8.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! It looks like Immutable refers to https://tc39.es/ecma262/#immutable-prototype-exotic-object ?

Comment on lines 192 to 195
EnumBody:
"A cover of enum bodies defined in [TypeScript](https://www.typescriptlang.org/docs/handbook/enums.html) and Flow.",
EnumMember:
"A cover of enum membors defined in [TypeScript](https://www.typescriptlang.org/docs/handbook/enums.html) and Flow.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think those are only for Flow

Function:
"A cover of functions and [method](#method)s, the must have `body` and `params`. Note: `Function` is different to `FunctionParent`.",
FunctionParent:
"A cover of AST nodes that start an execution context with new [VariableEnvironment](https://tc39.es/ecma262/#table-additional-state-components-for-ecmascript-code-execution-contexts). In other words, they define the scope of `var` declarations. FunctionParent did not include `Program` since Babel 7.",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the definition, I think it should also cover StaticBlock, which does define a new var scope. Note that in Babel 6 FunctionParent includes Program, but we removed Program in #5923 because the name FunctionParent strongly indicates that it should always be a Function, which is not the case for StaticBlock and Program.

Currently FunctionParent and Function are equivalent, they include exactly the same AST nodes.

BTW we have several usage of scope.getFunctionParent() || scope.getProgramParent(). This depends on the assumption that an AST tree can only have one Program node and all the function node must placed within that program node. This is not true for module-blocks proposal.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Babel 8 we might just rename them to LexicalScope and VariableScope, where VariableScope is a subset of LexicalScope.

This is not true for module-blocks proposal.

Can you show an example where scope.getFunctionParent() || scope.getProgramParent() would break with the module-blocks proposal? When I proposed to use Program for the ModuleExpression node it was exactly to support this existing pattern.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I missed that module blocks can only be at the top level.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, they can also be nested, but we'll want to consider "the top level" the module block itself.

For example, if we have

let mod = module {
  class A {}
};

We want to get this:

let mod = module {
  function _classCallCheck() {}
  let A = function A() {
    _classCallCheck(this);
  };
};

and not this:

function _classCallCheck() {}
let mod = module {
  let A = function A() {
    _classCallCheck(this);
  };
};

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I just checked module blocks proposal and you are right they can be nested.

Can you show an example where scope.getFunctionParent() || scope.getProgramParent() would break with the module-blocks proposal

function f() {
  (module {
    var x;
  })
}

scope.getFunctionParent() || scope.getProgramParent() on var x will return function f instead of the module block.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, thanks

TSType: "A cover of TypeScript type annotations.",
TSTypeElement: "A cover of TypeScript type declarations.",
Terminatorless:
"A cover of AST nodes whose semantic will change when a line terminator is inserted between the operator and the operand.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This description isn't true for await and throw, but I don't know what a better description would be 🤷

Copy link
Contributor Author

@JLHwung JLHwung Apr 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find the usage in our codebase. The term terminatorless is referenced in

/**
* Set some state that will be modified if a newline has been inserted before any
* non-space characters.
*
* This is to prevent breaking semantics for terminatorless separator nodes. eg:
*
* return foo;
*
* returns `foo`. But if we do:
*
* return
* foo;
*
* `undefined` will be returned and not `foo` due to the terminator.
*/
startTerminatorless(isLabel: boolean = false): any {

Based on that I don't think ThrowStatement and AwaitExpression should belong to TerminatorLess, we can also consider just remove it.

Expression:
"A cover of any [Expression](https://tc39.es/ecma262/#sec-ecmascript-language-expressions)s.",
ExpressionWrapper:
"A wrapper of expression that does not have run semantics.",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently ExpressionWrapper includes

ExpressionStatement
ParenthesizedExpression
TypeCastExpression

We have a very similar helper for the same purpose:

export function isTransparentExprWrapper(node: Node) {
return (
t.isTSAsExpression(node) ||
t.isTSTypeAssertion(node) ||
t.isTSNonNullExpression(node) ||
t.isTypeCastExpression(node) ||
t.isParenthesizedExpression(node)
);
}

this helper is used in optional chaining to stick chains separated by such transparent wrappers. Since an ExpressionStatement can not be nested inside expression, I think it is safe to merge these two, in other words we can consider to extend ExpressionWrapper to cover TSNonNullExpression, TSAsExpression and TSTypeAssertion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can either merge them, or just delete ExpressionWrapper and replace it with TransparentExpressionWrapper (which has the property that they don't modify the this context for call),

const aliasDescriptions = {
Binary:
"A cover of BinaryExpression and LogicalExpression, which share the same AST shape.",
Block: "A cover of BlockStatement and its alike.",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Block is not used by our codes. I am not sure about its intention.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just remove it in Babel 8

@JLHwung
Copy link
Contributor Author

JLHwung commented Apr 30, 2021

Merging as-is since it is a docs generator script and not published to npm.

@JLHwung JLHwung merged commit 9d6893d into babel:main Apr 30, 2021
@JLHwung JLHwung deleted the add-alias-docs-generator branch April 30, 2021 20:08
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jul 31, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Docs 📝 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants