Closed benmonro closed 8 years ago
Hi @benmonro, At first sight I don't see a good reason for additional code:
Factory
is equal to Textbox
(except for the getPlaceholder
method). export default (Component) => {
class ComponentFactory extends t.form.Textbox {
...
}
}
@gcanti the main reason is that the Component we are creating the factory for is not a Textbox, so why should we have to extend textbox. It makes the code confusing to someone new reading it. In addition, as you mentioned, it does not include the getPlaceholder. This component won't have a place holder, so again it can confuse any other developers that might come along and have to maintain this code.
Yes, names can be confusing when you go the custom factory route (but I can't think of better names).
Basically it depends on the type you want to handle:
this leads to the getComponent(type) -> Factory
function.
So when you want to define a custom factory for the type T
you should extend getFactory(T)
.
(but I can't think of better names)
I think Factory is a pretty good name. :)
Basically it depends on the type you want to handle:
In our case, the component can vary. It can be a text field or a selectbox or whatever the engineer wants to do w/ the field. I understand what you mean about mapping types to Factories, but in our case, the lines are not so clear cut. It can be an enum OR a string.
This pull request adds a generic 'Factory' which can be extended to more easily create a custom Factory. This means you no longer have to extend 'Textbox' (when it isn't a textbox) nor do you have to extend Component (which requires more work for validation, state, etc) An example of a generic Component factory might look like this: