Skip to content

Fix line break before ! after a comment in member chains - #19916

Open
kovsu wants to merge 9 commits into
prettier:mainfrom
kovsu:fix-non-null-after-trailing-comment
Open

kovsu wants to merge 9 commits into
prettier:mainfrom
kovsu:fix-non-null-after-trailing-comment

Conversation

@kovsu

@kovsu kovsu commented Aug 24, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Fixes #12529

Checklist

  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory).
  • (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/XXXX.md file following changelog_unreleased/TEMPLATE.md.
  • I’ve read the contributing guidelines.
  • I did not use AI to generate this PR.
  • (If the above is not checked) I have reviewed the AI-generated content before submitting.

@netlify

netlify Bot commented Aug 24, 2026

Copy link
Copy Markdown

✅ Deploy Preview for prettier ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 903ba2b
🔍 Latest deploy log https://app.netlify.com/projects/prettier/deploys/6a8ba4cf4394a100085a4141
😎 Deploy Preview https://deploy-preview-19916--prettier.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@pkg-pr-new

pkg-pr-new Bot commented Aug 24, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

yarn add https://pkg.pr.new/@prettier/[email protected]
yarn add https://pkg.pr.new/@prettier/[email protected]
yarn add https://pkg.pr.new/@prettier/[email protected]
yarn add https://pkg.pr.new/[email protected]

commit: 965c715

@netlify

netlify Bot commented Aug 24, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for prettier ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 965c715
🔍 Latest deploy log https://app.netlify.com/projects/prettier/deploys/6a8faea99cf87200081372ab
😎 Deploy Preview https://deploy-preview-19916--prettier.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@fisker

fisker commented Aug 24, 2026 •

Copy link
Copy Markdown
Member

I think the problem here is that we need print parenthesis for some cases.

Prettier PR #19916
Playground link

--parser typescript

Input:

(obj.aaa /*
c */)!.substring(0);

Output:

obj
  .aaa /*
c */!
  .substring(0);

Second Output:

SyntaxError: Expression expected. (4:3)
  2 |   .aaa /*
  3 | c */!
> 4 |   .substring(0);
    |   ^
  5 |

(followingNode?.type === "Identifier" ||
(enclosingNode.computed &&
precedingNode?.type === "TSNonNullExpression" &&
followingNode === enclosingNode.property))

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

main current
Image Image

Comment on lines +247 to +256
if (
currentGroup.length > 0 &&
isMemberExpression(printedNodes[i].node) &&
hasComment(printedNodes[i].node, CommentCheckFlags.Leading)
) {
groups.push(currentGroup);
currentGroup = [];
hasSeenCallExpression = false;
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

main current
Image Image

@kovsu

kovsu commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator Author

I think the problem here is that we need print parenthesis for some cases.

Fixed, see link

Comment thread src/language-js/print/typescript.js Resolved
return false;
}

if (shouldAddParenthesesToNonNullOperand(path, options)) {

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.

Let's move it to parentNeedsParentheses().

Comment thread src/language-js/print/member-chain.js Outdated
!needsParentheses(path, options)
!needsParentheses(path, options) &&
!path.call(
() => shouldAddParenthesesToNonNullOperand(path, options),

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.

Let's call needsParentheses() instead, maybe also check node.type, since we don't want print own comments for other nodes.

Comment thread src/language-js/print/index.js Outdated

doc = printCommentsForFunction(path, options, doc);

if (shouldAddParenthesesToNonNullOperand(path, options)) {

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.

This branch has not been deployed

No deployments
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

Successfully merging this pull request may close these issues.

Non-null assertion after long inline comment breaks parsing

2 participants