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

Add support for extension types #399

Closed
joshualitt opened this issue Mar 2, 2023 · 4 comments
Closed

Add support for extension types #399

joshualitt opened this issue Mar 2, 2023 · 4 comments
Assignees

Comments

@joshualitt
Copy link

I'd like to use inline classes in package:code_builder.

AFAICT there are two approaches we can take:

  1. Just add a bit to ClassBuilder.
  2. Create an entirely new builder just for inline classes.

(1) is more straightforward, and perhaps more discoverable for users. But inline classes are a bit more restricted than regular classes, so I can definitely see an argument for (2).

Thoughts?

CCing @kevmoo @natebosch @matanlurey for feedback.

@kevmoo
Copy link
Member

kevmoo commented Mar 2, 2023

My initial thought: this package isn't the analyzer. If folks generate invalid code, they will find out statically or at runtime pretty quick.

If it's 5% more work to make a separate thing, consider it. If it's 50% more work – or tough to discover, I'd just add a flag to the normal class.

Just my opinion. Happy to hear thoughts from others...

@natebosch
Copy link
Member

Agreed that we don't need to spend too much effort on keeping usage on the rails.

I don't have a strong opinion on whether it's a new class or a field on ClassBuilder.

@joshualitt
Copy link
Author

Okay, if people don't have strong feelings, and validation isn't that important, than I think it is easier to just make this a bit.

The only argument against making it a bit is that it could provide better validation for users. However, the cost of that benefit is pretty substantial. In addition to being more work to land the feature, it'd also be more work to maintain. Furthermore, there are other class modifiers, i.e. sealed, and those really only make sense as bits. Having inline be the odd modifier, even if some of the differences between inline and non-inline classes are pretty substantial, will probably be confusing to users.

Anyway, we have some time to think about this. I'll try and find the time to put together a CL in the next few weeks and we can discuss things further there.

@srujzs
Copy link
Contributor

srujzs commented Nov 14, 2023

I'm going to likely take a crack at this so that we can migrate package:web to extension types. I haven't dug into this package too much to determine if we should just modify ClassBuilder, but extension types are a sufficiently different feature that I'm going to investigate having a separate builder. We've also basically finalized the syntax for extension types, so now's a good time to add this to the package.

@srujzs srujzs self-assigned this Nov 14, 2023
@srujzs srujzs changed the title Add support for inline classes Add support for extension types Nov 14, 2023
srujzs added a commit to srujzs/code_builder that referenced this issue Nov 17, 2023
srujzs added a commit to srujzs/code_builder that referenced this issue Nov 17, 2023
srujzs added a commit to srujzs/code_builder that referenced this issue Nov 17, 2023
srujzs added a commit to srujzs/code_builder that referenced this issue Nov 20, 2023
@srujzs srujzs closed this as completed Nov 20, 2023
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

No branches or pull requests

4 participants