search

phetsims / axon

Axon provides powerful and concise models for interactive simulations, based on observable Properties and related patterns.
MIT License
11 stars 8 forks source link

Add ReadOnlyProperty #377

Closed pixelzoom closed 2 years ago

pixelzoom commented 2 years ago

This work was started without an issue, but seemed like we should have one, so here it is.

pixelzoom commented 2 years ago

@jonathanolson commited ReadOnlyProperty in the above commit, but there were some problems. See Slack discussion below.

Slack [Jonathan Olson](https://app.slack.com/team/U6E4HHSTF) [6:09 PM](https://phetsims.slack.com/archives/D6FNMP2LX/p1646356162283089) Added ReadOnlyProperty, see https://github.com/phetsims/axon/commit/2dc362b17edb24f6f465ef1ee46596eee1c071ce ... [Chris Malley](https://app.slack.com/team/U6EMFARV2) [6:44 PM](https://phetsims.slack.com/archives/D6FNMP2LX/p1646358291070669) Boo hoo. I still can’t pass a ReadOnlyProperty to addLinkedElement( element: PhetioObject, options?: any): void. For example FocalPointNode.ts constructor param pointProperty. [Jonathan Olson](https://app.slack.com/team/U6E4HHSTF) [6:47 PM](https://phetsims.slack.com/archives/D6FNMP2LX/p1646358441777559) Hmm, checking [Chris Malley](https://app.slack.com/team/U6EMFARV2) [6:47 PM](https://phetsims.slack.com/archives/D6FNMP2LX/p1646358453358009) addLinkedElement has been my pain point. [Jonathan Olson](https://app.slack.com/team/U6E4HHSTF) [6:48 PM](https://phetsims.slack.com/archives/D6FNMP2LX/p1646358485351349) Reproduced [Jonathan Olson](https://app.slack.com/team/U6E4HHSTF) [6:50 PM](https://phetsims.slack.com/archives/D6FNMP2LX/p1646358625407959) It's expecting... private/protected things? [Chris Malley](https://app.slack.com/team/U6EMFARV2) [6:51](https://phetsims.slack.com/archives/D6FNMP2LX/p1646358684354209) The fields that TS identifies as missing are protected/private? [Jonathan Olson](https://app.slack.com/team/U6E4HHSTF) [6:51 PM](https://phetsims.slack.com/archives/D6FNMP2LX/p1646358687340369) Yeah my ReadOnlyProperty is somehow very broken ![image](https://user-images.githubusercontent.com/3046552/157111493-e2ec555b-02e3-410c-afed-529361e89935.png) [Chris Malley](https://app.slack.com/team/U6EMFARV2) [6:53 PM](https://phetsims.slack.com/archives/D6FNMP2LX/p1646358811769059) I created https://github.com/phetsims/geometric-optics/issues/359 to remind myself to fix things up in GO when this is working. No rush. [Jonathan Olson](https://app.slack.com/team/U6E4HHSTF) [7:12 PM](https://phetsims.slack.com/archives/D6FNMP2LX/p1646359950887119) TypeScript is... killing me again. Not sure if this is possible. Omit (and the extends/infer it's based on) seems to convert it away from an interface into a type that loses the protected/private info
pixelzoom commented 2 years ago

In https://github.com/phetsims/scenery-phet/issues/726#issuecomment-1062117397, @jonathanolson said:

ReadOnlyProperty is buggy, and not ready for use. I'm not sure it's possible to make it work. Perhaps I should remove it?

Yes, probably so.

pixelzoom commented 2 years ago

It's been 30 days since we discussed deleting ReadOnlyProperty . If ReadOnlyProperty is not going to be fixed, can we please remove it? More than once, I've accidentally ended up with ReadOnlyProperty when I wanted IReadOnlyProperty.

samreid commented 2 years ago

I'll work on removing it momentarily.

samreid commented 2 years ago

Removed. Type checking and linting is passing locally. There were no usages. @pixelzoom can you please review?

pixelzoom commented 2 years ago

👍🏻