Dvc.org: js: refactor repetitive React components

[jorgeorpinel]

(and possibly elements, if any)

Could many of these be merged and use State/Props instead of repetition to apply contextual differences? Or made into a class hierarchy?

Example:

https://github.com/iterative/dvc.org/blob/4a849f56444a326c6d1e4255d5f556fc35b08b52/src/PromoSection/index.js#L16-L32

vs.

https://github.com/iterative/dvc.org/blob/4a849f56444a326c6d1e4255d5f556fc35b08b52/src/TrySection/index.js#L7-L22

vs.

https://github.com/iterative/dvc.org/blob/4a849f56444a326c6d1e4255d5f556fc35b08b52/src/Subscribe/index.js#L7-L20

not to mention lots and lots of styled components – Perhaps the easiest to begin with being the ones inside pages/*.js files.

[jorgeorpinel]

not to mention lots and lots of styled components

Example (exact copy):

https://github.com/iterative/dvc.org/blob/4a849f56444a326c6d1e4255d5f556fc35b08b52/src/TrySection/index.js#L29-L36
vs.
https://github.com/iterative/dvc.org/blob/4a849f56444a326c6d1e4255d5f556fc35b08b52/src/PromoSection/index.js#L34-L41

Example (very similar, could use props for differences):

https://github.com/iterative/dvc.org/blob/4a849f56444a326c6d1e4255d5f556fc35b08b52/src/TrySection/index.js#L102-L128
vs.
https://github.com/iterative/dvc.org/blob/4a849f56444a326c6d1e4255d5f556fc35b08b52/src/Subscribe/index.js#L53-L83

Also detected const Title and const Button in src/TrySection vs. the ones in src/PromoSection.

[jorgeorpinel]

cc @iAdramelk input please 🙂

[iAdramelk]

i think that a lot of existing components can be refactored/merged. But can't say beforehand which exactly should, examples above was created before me, and I'm not really familiar with them. In some cases some code repetition that add readability is better code without repetition, but which is harder to read.

[jorgeorpinel]

Thanks for the feedback Alexey. I mainly wanted to know whether others who have more familiarity with our React code agree with me on this (and seems like you do, in general).

In some cases some code repetition that add readability is better code without repetition, but which is harder to read.

I want to agree with this. But where is the line? The examples above seem too far into blind repetition (away from DRY). Let's see what @shcheklein thinks and what priority he assigns to this. 🙂

[shcheklein]

@jorgeorpinel it's hard to give a precise answer :) What I would recommend is to do this by small steps. First fix one obvious ugly repetition - that will force us develop some framework to handle this - some styled commons file or whatnot. We'll discuss and see if it makes code more manageable, readable and will come up with a better solution.

[shcheklein]

@jorgeorpinel priority is def not high - p-2. But give it a try if you want to use this as a framework to learn. Otherwise I would recommend taking one of may p-1s we already have.

[jorgeorpinel]

I'll go for p1 stuff. Addressing this wouldn't be very didactic for me, just stuff I'm noticing as I work more with our Rect code.

[shcheklein]

I think it is significantly improved after @pavelgrinchenko 's changes. Closing this for now. Let's open ticket with specific suggestions if we still have duplicates.