Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use accessors for readonly properties #1165

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Dionysusnu
Copy link
Contributor

@Dionysusnu Dionysusnu commented Feb 6, 2024

This resolves the workaround where properties that are read access None but write access PluginSecurity had to be marked writable in None.d.ts to avoid the TS error "All declarations of 'property' must have identical modifiers. (2687)".

We can actually specify these in None.d.ts with a get accessor. When a set accessor is missing, that makes the property readonly. We specify the set accessors in PluginSecurity.d.ts, so that when plugin types are enabled, they make it a regularly accessible property.

@Dionysusnu Dionysusnu marked this pull request as draft February 6, 2024 08:25
@Dionysusnu Dionysusnu force-pushed the get-accessor-readonly-properties branch from 66c7f26 to 44e8551 Compare February 6, 2024 08:42
@Dionysusnu Dionysusnu marked this pull request as ready for review February 6, 2024 08:43
@osyrisrblx
Copy link
Member

This seems like a good idea for me. Can we keep the readonly for properties which aren't writable in any security level?

@Dionysusnu
Copy link
Contributor Author

I did consider that, but it seems quite tricky to do with the way the types generation is laid out. It would be a hack, or a lot of work. Is there any benefit to using readonly, apart from less diff in the PR? I tested and it seems to work the same with just the get() accessor.

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