css: migrate from styled comps to plan css or css-modules

[jorgeorpinel]

Extracted from #776 (comment)

Our styled components tend to be defined after the exported component in each file (pages/, src/) so it could be confusing to find them in the JSX of previously defined components.

However we want to keep them in the bottom of each file given their strange code format. The idea then is to RENAME them so it's more obvious that they are styled components. My proposal is to simply add the name of the HTML tag they use in CamelCase. For example a Container component that styles a div tag would be renamed ContainerDiv and look like this in JSX.

UPDATE: Probably going for extracting and not renaming. See #792 (comment)

[shcheklein]

I would check if there are best practices around this. Also, would love to know @iAdramelk 's take on this.

[jorgeorpinel]

I don't think styled-components.com are so used that there will be broadly established practices around it but I did look this up just now and found https://medium.com/inturn-eng/naming-styled-components-d7097950a245 where they propose using StyledContainer or S.Container, which is what Alexey suggested in #776 (comment). But I still like ContainerDiv more. Let's just vote on it?

[jorgeorpinel]

• BTW if we do this and rename the Link styles components to something else, we should probably rename NextLink to just Link (it's natural name from next/link).

[jorgeorpinel]

Oh, and another option to consider for all this is to migrate to https://github.com/zeit/styled-jsx (supported out-of-box in Next.js) but doesn't support nested styles I think.

[jorgeorpinel]

Do you have a vote here, @iAdramelk? Thanks

[iAdramelk]

@jorgeorpinel I do think that we should probably move them to the separate files at least. I started experimenting with this approach in #874 (check the files in src/Community) and I think that this way it is a lot more convenient. We can rename them too, but if we move them to the separate files it is not that important, because it is easier to understand that they are a part of the styles. Alternatively, naming convention can be to add prefix to them. Like SCLink instead of Link. This way we will not lock ourselves in using predefined tags there and will be able to change tags without changing component names.

I checked styled-jsx and I am not a big fan of it, it looks janky to me. But I very much want to replace styled-comonents with something that not requires runtime and builds to static css files, like https://github.com/4Catalyzer/astroturf . Right now styled-components is making our bundle size unnecessary large and slows down pages render speed (and also make react profiling tools almost impossible to use). Alternatively, latest version of Next has added css-modules support so we can just switch to normal css instead (old versions of Next didn't work with normal css correctly)

[jorgeorpinel]

We can rename them too, but if we move them to the separate files it is not that important, because it is easier to understand that they are a part of the styles.
Example: https://github.com/iterative/dvc.org/pull/874/files?file-filters%5B%5D=.js#diff-6f1eb6a2d283c5cf998903a810e9f7a3

Agree. Probably no need to rename them if they're moved. I'd call the source file styled.js BTW (left a comment in your PR).

add prefix to them. Like SCLink instead of Link. This way we will not lock ourselves

I don't really see the need. There's no locking since you can always rename them at import. I don't love any of the prefixes or suffixes we've suggested here anyway.

[jorgeorpinel]

I checked styled-jsx ... looks janky. But I very much want to replace styled-comonents with something ... builds to static

Well, styled-jsx comes from the same developers as Next.js haha...
UPDATE: More officially recommended options: https://github.com/zeit/next.js#css-in-js

Agree about substituting styled-components eventually, but probably needs a separate issue? Unless you think it's a high priority (cc @shcheklein) because that would basically render this and possibly other issues no-longer-relevant.

Rewriting with https://github.com/css-modules/css-modules sounds like the easiest option to me at this point, BTW. But we can list and discuss the options in a new issue if you guys agree.

[fabiosantoscode]

Hey, https://emotion.sh is a thing.

It has all the features of styled-components, plus it can easily give you class names that you can pass to random libraries on NPM, react or not. It's more likely that you can customise a className than that you're able to control the element generation just to change a style.

Plus it has a css prop thing (granted, it requires a babel plugin) that allows you to define the styles right in the divs and spans and whatever which use them.

Worth a look?

[shcheklein]

@fabiosantoscode @iAdramelk is migrating dvc.org to Gatsby now and probably we'll use the same convention eventually (postcss?) . @iAdramelk could you shed some light here?

[iAdramelk]

@fabiosantoscode thanks for suggestion, but the thing is, we didn't really need css-in-js library. We don't use any of it's advanced capabilities and in our case just importing css without the need for a runtime will be a lot better for the perfomance. The main reason why we use styled-components is because next.js can't work with plain css or css-modules. But we are now migrating to gatsby which didn't have this problem, and even if we didn't migrate, next version of next.js will support css-modules out of the box.

[jorgeorpinel]

@iAdramelk @shcheklein so is this issue still valid? If the Gatsby migration is definitive, let's close it?

[shcheklein]

let's keep, we'll close all the tickets with the engine label (those that will become obsolete) at once after the migration. Since it's not over, the issue is still relevant.

[shcheklein]

Fixed by the recent migration.