feat(@clayui/core): Add Provider component and the new @clayui/core package

[matuzalemsteles]

Fixes #2353

This commit adds the initial implementation of the @clayui/core package which will aggregate all Clay components during the next releases of v3, and v4 we will be stopping publishing packages separately if the separate packages still have a lot of value, we will discuss further. Bryce had an initial brainstorm on #3966.

When exporting components, we are adopting semantics without adding the Clay prefix to the component names so that the library can be more harmonious within an application and eliminate noise in the JSX markup views. This might look like a breaking change but I'm not changing this on packages just in the export in the core package.

Unlike the #3966 implementation, I haven't added all the packages yet in the core package, but I plan to do that in the next few days, we also don't export the types yet we have to handle it better but the types will work. As in future versions, it is to aggregate the packages in the core, I also added a minimal structure to follow with the other packages, this is similar with some other libraries that have the same purpose of exporting all components in the same package.

clay-core/
├─ src/
│  ├─ provider/
│  │  ├─ src/
│  │  │  ├─ Provider.tsx
│  │  ├─ stories/
│  │  │  ├─ Provider.stories.tsx
│  │  ├─ index.ts
│  ├─ index.ts

Unlike what we do with current packages, we mix index.js to export components and also add components to it, in this structure we want to use index only for file export to facilitate import/export of module components and also to facilitate the search, the stories also follows the same naming in order to facilitate the search by components and to differentiate what is a story and a component.

If necessary, I can describe an RFC to adopt this as a pattern for the next components that will be added to core.

We are also adding the new Provider component which aggregates the main Clay, Icon, and Modal Contexts but we have not added Link and Tooltip contexts because both are commonly used in certain parts of the application structure, the Tooltip is used as a Global component in DXP, so we're discarding here to avoid conflicts. It also implements the theme API as discussed in liferay-frontend/liferay-portal#1194 (comment).

[julien]

@matuzalemsteles just a question, isn't it a bit early to talk about v4?
AFAIK, we haven't had any discussion about this, and I think that this time it would be nice to involve as much people as we can, not just those from the clay squad.

[matuzalemsteles]

just a question, isn't it a bit early to talk about v4?

Hey @julien I think we've been talking about the next major version for some time in some issues and PRs, we're accumulating changes for the next major version, see the BREAKING.md files in each package.

• Label
• Modal
• MultiSelect
• Nav
• Card

It's also natural to think about the next major version, v3 was released almost 2 years ago, so we wanted to maybe shorten those major versions to enable us to ship more changes and refinements and new improvements of things that didn't work out earlier, instead of accumulating a lot APIs, probably following the same release time as DXP. This is natural in some open source software but of course, we want to be careful with this but Clay is starting to get into the maturity process, we figured out what doesn't work in v3 and what we need to fix. It's also worth remembering that a Major Version doesn't mean doing a complete Clay rewrite like we did from v2 to v3, this was a technology change exception. So we can have major versions just deprecating some APIs, behaviors...

The Squad should take responsibility for this now, adding other people to the conversation is always welcome, just as we did at the beginning of V3 #2017 development, also the RFC process input is an incentive for that #4126 🙂.

Regarding this change, as I had commented @bryceosterhaus had started work after we talked about it in #3940 raising by Greg and I think we all agreed with the one-package idea. See more speeches here too #3966.

The idea of bringing this back is that we need a new Provider component to aggregate the providers into one and also solve a problem with cadmin isolation and we also want to avoid creating new packages.

cc @pat270 see the Storybook example for usage.

[julien]

Hey @matuzalemsteles thanks for that information.
I reads the threads you mentioned (#3940, #3966) which helped me understand the intent of these changes.

I also fetched this branch and when building I'm getting warnings from lerna which look like

lerna WARN ECYCLE Dependency cycles detected, you should fix these!

Not sure if that intended, but in any case the build works as expected and the storybook examples work just fine.

I checked the "CI" errors and wasn't able to figure out why yarn run sizeLimit failed, locally it passes just fine, we might also need to tweak the checkDeps script to take to clay-core package into account (or ignore it) but other than that, this looks good to me.

[matuzalemsteles]

@julien no problem. Yes, yesterday I was looking at this, I found it strange that checkDeps fails, I did a clean repo and ran it locally and it seems to fail, for some reason when I run yarn lerna build it is not compiling all the packages, I will check again today that might have happened, it might be related to the error you pointed out above, I'll check that too.

[matuzalemsteles]

I investigated this a bit today and it actually relates to the cyclic dependency between @clayui/core and @clayui/shared, that's because the ClayPortal component that is used internally is using the Provider to add the theme scope which ultimately is generating a cyclic dependency between packages that @clayui/core depends on and that they depend on @clayui/shared.

I thought about moving some components but the problem will still persist. Maybe the only solution for now is to create a new package @clayui/provider just to solve the cyclic dependency problem, maybe we don't even need to publish this package, just keeping it private I don't know if that works but it would be the only way to avoid it that I can imagine now, I wanted to avoid creating a new package but 🤷‍♂️. Maybe for DXP use we can encourage just using @clayui/core instead of directly using @clayui/provider until we merge all in one package.

[julien]

Hey @matuzalemsteles thanks for the update - if creating a private package solves the issue it sounds like a good idea, if I have time to have a look too I will.

[matuzalemsteles]

Yesterday I made some advances, one of the solutions to alleviate this problem is to add @clayui/core as peerDependencies in the shared package because we need it there to consume the Provider.

I just tested it by creating a new package for Provider but still the problem persists since Provider has Modal and Icon dependencies. A fact that hurts now is that we have a peerDependencies of core in shared, all packages end up having this dependency that increases the size, in some packages it increased 50% of the size because we didn't add all the packages in @clayui/core, so I'm assuming this value can increase further.

Maybe it's not so much worth keeping Provider inside core for now, I'll just add one new package for that purpose and add it as peerDependencies in shared so we have a controlled size.

[matuzalemsteles]

It looks safe now, we've averaged a 7% increase in package sizes compared to 50%. This is failing because we have a 5% quota, for now I'm going to ignore that so we can merge this.

Although we have the @clayui/provider package we are not encouraging its direct use, instead we recommend using @clayui/core making this explicit in the documentation and README.md.

The core package isn't 100% complete yet because I haven't added all the packages, so far I've only added a few packages, we won't upload it yet to DXP but we'll keep it around to add new packages in core and we do more testing.

[matuzalemsteles]

Well, since the @clayui/core component is just being a package aggregator at the moment, I'll work on an RFC later to add the reasons and further raise the discussion of how we should organize the components within the core package so that patterns, motivation, etc. are recorded.

I'm going to merge that so we have the Provider in this release and after the DXP release, we can start replacing the direct use of packages with @clayui/core to make our release easier and simplify everything.