FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Avoid extra parens in TS `as`/`satisfies` by nicolo-ribaudo · Pull Request #16617 · babel/babel · GitHub

/ babel Public

Avoid extra parens in TS as/satisfies - #16617

Merged
nicolo-ribaudo merged 1 commit into
babel:mainfrom
nicolo-ribaudo:ts-unnecessary-parens
Jul 12, 2024
Merged

nicolo-ribaudo merged 1 commit into
babel:mainfrom
nicolo-ribaudo:ts-unnecessary-parens

Conversation

Copy link
Copy Markdown
Member
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

We were being extra conservative and always printing parens in our code generator. This PR avoids it, by considering as/satisfies as having the same precedence on the left as in. Extra care is needed for |/&, since they are different operators at the type and value levels.

nicolo-ribaudo added PR: Polish 💅 A type of pull request used for our changelog categories pkg: generator pkg: types labels Jul 5, 2024
a | b as c;
a as b | c;
(a as b) | c;
a as (b | c);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Here parens are not actually necessary, but our AST has a TSParenthesizedType node so they are preserved. Babel 8 removes the parens, because TSParenthesizedType is not enabled by default anymore.

nicolo-ribaudo force-pushed the ts-unnecessary-parens branch from c8c3390 to ceff568 Compare July 5, 2024 17:53

babel-bot commented Jul 5, 2024
edited
Loading

Copy link
Copy Markdown
Collaborator

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

liuxingbaoyu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This is awesome!

Comment on lines +48 to +59
function getBinaryPrecedence(node: t.Node) {
if (node.type === "BinaryExpression" || node.type === "LogicalExpression") {
return PRECEDENCE.get(node.operator);
}
if (node.type === "TSAsExpression" || node.type === "TSSatisfiesExpression") {
return PRECEDENCE.get("in");
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can we add a parameter nodeType to use the parentType of the caller?

nicolo-ribaudo force-pushed the ts-unnecessary-parens branch from 6ac192d to f083dbe Compare July 6, 2024 15:08

Copy link
Copy Markdown
Member

I noticed this PR was tagged pkg: types and there doesn't seem to be any relevant changes?

Copy link
Copy Markdown
Member Author

by mistake :)

nicolo-ribaudo merged commit 3e4723f into babel:main Jul 12, 2024
nicolo-ribaudo deleted the ts-unnecessary-parens branch July 12, 2024 19:37

cpojer commented Jul 16, 2024
edited
Loading

Copy link
Copy Markdown
Member

The latest Babel version is causing breakage in Athena Crisis, and I believe this PR is related. Specifically it's breaking the common patterns below:

Spread is somehow broken due to the as const

export const MoveAction = (
  from: Vector,
  to: Vector,
  path?: ReadonlyArray<Vector> | null,
  complete?: boolean,
) =>
  ({
    ...(complete ? { complete } : null),
    from,
    ...(path ? { path } : null),
    to,
    type: 'Move',
  }) as const;

Source: https://github.com/nkzw-tech/athena-crisis/blob/main/apollo/action-mutators/ActionMutators.tsx#L5-L17

Type casting a function isn't working

export default (function lazy(factory) {
  return _lazy(() => factory().catch(importErrorHandler));
} as typeof _lazy);

Source: https://github.com/nkzw-tech/athena-crisis/blob/main/ui/lib/lazy.tsx#L13-L15

The latter works with as (typeof _lazy), however prettier will strip the extraneous parenthesis.

To repro you can clone the linked repo, run pnpm up -r --latest and then run pnpm vitest tests/__tests__/CreateBuildingFog.test.tsx which should show you a number of "Pre-transform error" messages. I can't tell if this is because of the pipeline (React → Babel → Vite → esbuild/swc) or if this is also an issue with Babel only but the setup is fairly common.

github-actions Bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 16, 2024
github-actions Bot locked as resolved and limited conversation to collaborators Oct 16, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area: typescript outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: generator PR: Polish 💅 A type of pull request used for our changelog categories

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants


Back | FazBrowse Home | New Git URL