search

ignitionworks / draftjs_exporter

Export Draft.js content state into HTML.
MIT License
16 stars 20 forks source link

Require Link entity on entity_state.rb #1

Closed pcofilada closed 8 years ago

pcofilada commented 8 years ago

I think you forgot to require Link entity on your entity_state.rb.

theozaurus commented 8 years ago

Hi @pcofilada,

Thanks for the pull request. The require for link entity is not needed as the EntityState class does not have any knowledge of the Entities::Link constant. This is deliberate as the EntityState is now said to be open for extension (open/closed principal). It achieves this using dependency injection, which is a fancy way to say that it's dependencies are passed into it, as opposed to hardcoded.

This means that new entities could be added (as part of this gem, or part of the project the gem is being used in) without having to modify the EntityState class. It also means that if people do not want to use the link entity, they do not even have to load it.

Did you run into a problem with the example code as it does not show any requires? The expected way to do this is to add the requires where the DraftjsExporter::HTML is being configured.

Does this make sense to you? I'd totally accept a PR if you wanted to update the documentation to make it easier for people to get started.

Thanks for your time,

Theo

pcofilada commented 8 years ago

Hello @theozaurus,

That would make sense. I will just close this PR and make a separate PR for updating the documentation.

Thanks, Patrick