Skip to content

Conversation

@liuxingbaoyu
Copy link
Member

@liuxingbaoyu liuxingbaoyu commented Sep 1, 2025

Q                       A
Fixed Issues? Fixes #1, Fixes #2
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

Only the last commit is new.
Once the initial review is complete, I'll move it to Babel 8 only.

export interface MemberExpressionComputed extends BaseNode {
  type: "MemberExpression";
  object: Expression | Super;
  optional?: boolean | null;
  computed: true;
  property: Expression;
}
export interface MemberExpressionNonComputed extends BaseNode {
  type: "MemberExpression";
  object: Expression | Super;
  optional?: boolean | null;
  computed: false;
  property: Identifier | PrivateName;
}
export type MemberExpression =
  | MemberExpressionComputed
  | MemberExpressionNonComputed;
export function memberExpression(
  object: t.Expression | t.Super,
  property: t.Expression,
  computed?: true,
  optional?: boolean | null,
): Extract<t.MemberExpression, { computed: true }>;
export function memberExpression(
  object: t.Expression | t.Super,
  property: t.Identifier | t.PrivateName,
  computed?: false,
  optional?: boolean | null,
): Extract<t.MemberExpression, { computed: false }>;
export function memberExpression(
  object: t.Expression | t.Super,
  property: t.Expression | t.Identifier | t.PrivateName,
  computed?: boolean,
  optional?: boolean | null,
): t.MemberExpression;
export function memberExpression(
  object: t.Expression | t.Super,
  property: t.Expression | t.Identifier | t.PrivateName,
  computed: boolean = false,
  optional: boolean | null = null,
): t.MemberExpression {
  const node = {
    type: "MemberExpression",
    object,
    property,
    computed,
    optional,
  } as t.MemberExpression;
  const defs = NODE_FIELDS.MemberExpression;
  validate(defs.object, node, "object", object, 1);
  validate(defs.property, node, "property", property, 1);
  validate(defs.computed, node, "computed", computed);
  validate(defs.optional, node, "optional", optional);
  return node;
}

@babel-bot
Copy link
Collaborator

babel-bot commented Sep 1, 2025

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

@JLHwung
Copy link
Contributor

JLHwung commented Sep 5, 2025

@liuxingbaoyu Could you rebase? Thank you.

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

Very nice!

NODE_FIELDS,
NODE_UNION_SHAPES__PRIVATE,
toBindingIdentifierName,
} = _t as typeof import("@babel/types");
Copy link
Member

Choose a reason for hiding this comment

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

Could we instead say in tsconfig.json that imoprts to that lib/index.js should be mapped to the TS files for the type definitions?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried, but without success.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Sep 7, 2025

Open in StackBlitz

commit: dec95e9


/**
* Generate the builder arguments for a given node type.
* @param type AST Node type
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a short description for the declare parameter?

Copy link
Contributor

@JLHwung JLHwung left a comment

Choose a reason for hiding this comment

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

Great job!

Comment on lines 101 to 107
// Future / annoying TODO:
// MemberExpression.property, ObjectProperty.key and ObjectMethod.key need special cases; either:
// - convert the declaration to chain() like ClassProperty.key and ClassMethod.key,
// - declare an alias type for valid keys, detect the case and reuse it here,
// - declare a disjoint union with, for example, ObjectPropertyBase,
// ObjectPropertyLiteralKey and ObjectPropertyComputedKey, and declare ObjectProperty
// as "ObjectPropertyBase & (ObjectPropertyLiteralKey | ObjectPropertyComputedKey)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Has this todo item been done in this PR? Ditto for line 46.

@liuxingbaoyu
Copy link
Member Author

The CI failure is related and is being investigated!

@liuxingbaoyu
Copy link
Member Author

Due to #11384, I ended up not using \u{10FFFF}.
Because it's already parsed in rollup-plugin-dts, I was worried that users would run into the same issue when bundling @babel/types.

@liuxingbaoyu liuxingbaoyu added pkg: types PR: Polish (next major) 💅 A type of pull request used for our changelog categories for next major release labels Sep 8, 2025
@liuxingbaoyu liuxingbaoyu changed the title Better node type definitions for computed [Babel 8] Better node type definitions for computed Sep 8, 2025
return withStartEnd({
type: "Identifier",
- name: node.getText(),
+ name: text.startsWith(`"`) ? JSON.parse(text) : text,
Copy link
Member

Choose a reason for hiding this comment

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

Can we mark the field as @internal with TSDoc, and make it not appear in the generated .d.ts?

Copy link
Member Author

Choose a reason for hiding this comment

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

It worked, thanks!

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

(don't forget to make this babel-8-only)

/**
* @internal
*/
export { NODE_UNION_SHAPES__PRIVATE as "PRIVATE!!! NODE_UNION_SHAPES" };
Copy link
Contributor

Choose a reason for hiding this comment

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

If the @internal works, maybe we don't need the export name hack and can just keep the NODE_UNION_SHAPES__PRIVATE name?

@liuxingbaoyu
Copy link
Member Author

(don't forget to make this babel-8-only)

I did so in 1072c2d (#17500). :)

@nicolo-ribaudo nicolo-ribaudo merged commit 26bc651 into babel:main Sep 12, 2025
76 checks passed
@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 Dec 13, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 13, 2025
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 pkg: types PR: Polish (next major) 💅 A type of pull request used for our changelog categories for next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants