Skip to content

TypeScript: Add type null to Item properties#53

Open
danielrentz wants to merge 1 commit into
rubenv:masterfrom
danielrentz:patch-1
Open

TypeScript: Add type null to Item properties#53
danielrentz wants to merge 1 commit into
rubenv:masterfrom
danielrentz:patch-1

Conversation

@danielrentz

Copy link
Copy Markdown

As can be verified in source code, the properties Item#msgctxt and Item#msgid_plural will be initialized to null instead of undefined (https://github.com/rubenv/pofile/blob/master/lib/po.js#L279-L281). However, type undefined should remain to not break any existing code.

As can be verified in source code, the properties `Item#msgctxt` and `Item#msgid_plural` will be initialized to `null` instead of `undefined` (https://github.com/rubenv/pofile/blob/master/lib/po.js#L279-L281). However, type `undefined` should remain to not break any existing code.
@rubenv

rubenv commented Feb 24, 2023

Copy link
Copy Markdown
Owner

Shouldn't the question mark be dropped when you explicitly add | undefined?

@danielrentz

danielrentz commented Feb 24, 2023

Copy link
Copy Markdown
Author

I followed other projects here that explicitly added the "undefined" type to optional props withregards to compiler option "exactOptionalPropertyTypes", e.g. DefinitelyTyped/DefinitelyTyped#54256

The question mark is needed if class Item should support property deletion. I think this is unusual and may be dropped, but thinking strictly it would be a breaking change.

So I would be happy to adapt the PR to remove the question marks if you like.

@danielrentz

Copy link
Copy Markdown
Author

Anybody around here in this project?

@danielrentz

Copy link
Copy Markdown
Author

@rubenv: Did you lose interest in this project?

@rubenv

rubenv commented Mar 24, 2025

Copy link
Copy Markdown
Owner

@danielrentz I am not currently using it in any projects and work does not grant me much time to maintain it. Apologies.

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.

2 participants