GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Readonly support for jsdoc by sandersn · Pull Request #35790 · microsoft/TypeScript

Conversation

Nathan Shively-Sanders (sandersn) commented Dec 20, 2019

Copy link

Copy Markdown

Member

Add @readonly to jsdoc; works the same way readonly does in TS.

Fixes #17233

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Nathan Shively-Sanders (sandersn) added 4 commits

December 18, 2019 14:53


          Add @readonly

f7808c4

The rule for @readonly on this-assignments in the constructor is wrong.
See failing tests.


          In-progress

8974fb5

Add ctor function test
Add some notes and rename variable


          Done except for cleanup and fix 1 bug

55b48d4


          Fix last test and clean up

a1d3a3e

Nathan Shively-Sanders (sandersn) commented

Dec 20, 2019

View reviewed changes

Comment thread

src/compiler/checker.ts

		error(right, Diagnostics.Cannot_assign_to_0_because_it_is_a_read_only_property, idText(right));
		return errorType;
		}
		if (isAssignmentToReadonlyEntity(node as Expression, prop, assignmentKind)) {

Nathan Shively-Sanders (sandersn) Dec 20, 2019

Copy link

Copy Markdown

Member Author

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.

I collapsed these three checks into one function because they are always called together.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

andrewbranch reacted with thumbs up emoji

All reactions

👍 1 reaction

Daniel Rosenwasser (DanielRosenwasser) added the Update Docs on Next Release Indicates that this PR affects docs label

Dec 20, 2019

Nathan Shively-Sanders (sandersn) requested review from Andrew Branch (andrewbranch) and Wesley Wigham (weswigham)

December 20, 2019 16:38

Andrew Branch (andrewbranch) approved these changes

Dec 20, 2019

View reviewed changes

Comment thread

src/compiler/checker.ts

		const isLocalPropertyDeclaration = ctor.parent === symbol.valueDeclaration.parent;
		const isLocalParameterProperty = ctor === symbol.valueDeclaration.parent;
		const isLocalThisPropertyAssignment = isAssignmentDeclaration && symbol.parent?.valueDeclaration === ctor.parent;
		const isLocalThisPropertyAssignmentConstructorFunction = isAssignmentDeclaration && symbol.parent?.valueDeclaration === ctor;

Andrew Branch (andrewbranch) Dec 20, 2019

Copy link

Copy Markdown

Member

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 one is this.x = whatever inside a JS constructor function?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Nathan Shively-Sanders (sandersn) Dec 20, 2019

Copy link

Copy Markdown

Member Author

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.

Yep. Any suggestions for making the variable name clearer?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Andrew Branch (andrewbranch) Dec 20, 2019

Copy link

Copy Markdown

Member

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.

- isLocalThisPropertyAssignmentConstructorFunction
+ isLocalThisPropertyAssignmentInConstructorFunction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/compiler/checker.ts

		const isAssignmentDeclaration = isBinaryExpression(symbol.valueDeclaration);
		const isLocalPropertyDeclaration = ctor.parent === symbol.valueDeclaration.parent;
		const isLocalParameterProperty = ctor === symbol.valueDeclaration.parent;
		const isLocalThisPropertyAssignment = isAssignmentDeclaration && symbol.parent?.valueDeclaration === ctor.parent;

Andrew Branch (andrewbranch) Dec 20, 2019

Copy link

Copy Markdown

Member

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 would be true inside any method, but we’ve already ensured we’re in a constructor 👍

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Wesley Wigham (weswigham) approved these changes

Dec 20, 2019

View reviewed changes

Wesley Wigham (weswigham) left a comment

Copy link

Copy Markdown

Member

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.

How's the user suite look?

TypeScript Bot (@typescript-bot) user test this

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Nathan Shively-Sanders (sandersn) commented Dec 20, 2019

Copy link

Copy Markdown

Member Author

TypeScript Bot (@typescript-bot) user test this

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

TypeScript Bot (typescript-bot) commented Dec 20, 2019 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Heya Nathan Shively-Sanders (@sandersn), I've started to run the parallelized community code test suite on this PR at a1d3a3e. You can monitor the build here. It should now contribute to this PR's status checks.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Nathan Shively-Sanders (sandersn) commented Dec 20, 2019

Copy link

Copy Markdown

Member Author

Wesley Wigham (@weswigham) Looks like the bot doesn't pay attention to sign off comments.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

TypeScript Bot (typescript-bot) mentioned this pull request

Dec 20, 2019

🤖 User test baselines have changed for readonly-jsdoc #35808

Closed

TypeScript Bot (typescript-bot) commented Dec 20, 2019

Copy link

Copy Markdown

Contributor

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Nathan Shively-Sanders (sandersn) commented Dec 20, 2019

Copy link

Copy Markdown

Member Author

Results are the same as yesterday's @private results.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Nathan Shively-Sanders (sandersn) merged commit 3d2b92c into master

Dec 20, 2019

Klaus Meinhardt (ajafff) mentioned this pull request

Dec 26, 2019

handle JSDoc visibility modifier fimbullinter/wotan#671

Closed

Jake Bailey (jakebailey) deleted the readonly-jsdoc branch

November 7, 2022 17:34

Microsoft (microsoft) locked as resolved and limited conversation to collaborators

Oct 21, 2025

TypeScript Bot (typescript-bot) assigned Nathan Shively-Sanders (sandersn)

Oct 21, 2025

Reviewers

Andrew Branch (andrewbranch)

andrewbranch approved these changes

Wesley Wigham (weswigham)

weswigham approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

Nathan Shively-Sanders (sandersn)

Labels

Update Docs on Next Release

Indicates that this PR affects docs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

JSDoc: Support @readonly tag

Uh oh!

There was an error while loading. Please reload this page.

5 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

Nathan Shively-Sanders (sandersn) commented Dec 20, 2019

Uh oh!

Nathan Shively-Sanders (sandersn) Dec 20, 2019

Choose a reason for hiding this comment

Uh oh!

Andrew Branch (andrewbranch) Dec 20, 2019

Choose a reason for hiding this comment

Uh oh!

Nathan Shively-Sanders (sandersn) Dec 20, 2019

Choose a reason for hiding this comment

Uh oh!

Andrew Branch (andrewbranch) Dec 20, 2019

Choose a reason for hiding this comment

Uh oh!

Andrew Branch (andrewbranch) Dec 20, 2019

Choose a reason for hiding this comment

Uh oh!

Wesley Wigham (weswigham) left a comment

Choose a reason for hiding this comment

Uh oh!

Nathan Shively-Sanders (sandersn) commented Dec 20, 2019

Uh oh!

TypeScript Bot (typescript-bot) commented Dec 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nathan Shively-Sanders (sandersn) commented Dec 20, 2019

Uh oh!

TypeScript Bot (typescript-bot) commented Dec 20, 2019

Uh oh!

Nathan Shively-Sanders (sandersn) commented Dec 20, 2019

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

TypeScript Bot (typescript-bot) commented Dec 20, 2019 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.