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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proof of concept] Component isolation #231

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cllns
Copy link
Member

@cllns cllns commented Feb 16, 2022

dry-system provides a nice container for creating new instances of classes, but it doesn't enforce its use. Developers are free to create an object by calling its constant constructor (e.g. Foo.new)

...until now... 馃槺

With this proposed 'isolation' feature, a component's constant would be removed right after it's loaded, so the users would not be able to call Foo.new. Or call Foo at all. That is, they would have to use the container to create new instances.

This may be a very bad idea... it may be going too far. It may have negative unintended consequences. I could see it being useful in some circumstances though. The idea is to remove more global state, by having fewer constants.

This is clearly just a proof of concept. It could be a post-1.0 feature, too. If we wanted to implement this for real, some things (among more, I'm sure) we'd have to consider:

  • How would this be configured? Per component, per component_dir, per container?
  • Should this be a custom Loader? i.e. provided as a subclass of Loader, either in this gem or as an external one
  • Is this the right place to do this?
  • Is isolate/isolation the right name?
  • Do we want to go further, and load the class definitions once and store them internally somehow, and remove the constants then, once?

@cllns cllns requested a review from timriley February 16, 2022 22:32
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.

None yet

1 participant