Update to version 1.x

[asci]

UI kit is in production already for some time. A lot of widgets and core's components rely on it. In order to maintain the healthy state of the project would be great if we adopt SemVer.

There is already a PR with breaking changes and if we merge it — we will break some functionality on our website. And there more PRs are coming. Breaking API is not something we can expect from UI library. It should be stable, it should be trustworthy.

We can't go forward without breaking changes, but it should be predictable, and SemVer could provide the methodology for this predictability:

1. Major: Breaking changes
2. Minor: Add new feature
3. Patch: Bugfix

Discuss.

[fahad19]

I would highly recommend adopting a quarterly backwards-compatible release cycle. so that all developers get enough time to do the migration.

• Q1: deprecate features
• Q2: give time to do the migration
• Q3: drop the feature

if the migration happens early, then no reason to wait for the quarter to finish. just go ahead with dropping the feature.

[mAiNiNfEcTiOn]

Current state (< 1.0)

Honestly, just because it's live it doesn't mean it's stable. The following statement, by @asci, states that clearly

There is already a PR with breaking changes and if we merge it — we will break some functionality on our website. And there more PRs are coming. Breaking API is not something we can expect from UI library. It should be stable, it should be trustworthy.

Nevertheless, here's what I think we should define in order to release 1.0:

• The contract with the end-users. The end-users will interact with our UI Kit under which form? How will they feel a breaking change? How should they request a feature and have it available? Should we consider new components new major versions, even if it doesn't break contracts?

• The release cycle. As @fahad19 mentioned, we should have a periodical, predictable release cycle so that users can expect new versions/features and make migration code to move to the newer version. This includes: deprecation warnings, temporary support for new and old code and, later on, code removal.

• Standardise the issues and pull requests levels/labels. Labels should be used to prioritise the issues, and separate pull requests when it comes to releases, or reviews. I know that everyone thinks their PR's are important, but when it comes to releases we need to have a certain structure on it.

• Automate releases. Think on admins' dependency on releases. Would it be possible to release based on a label on the PR, so that if we merge a PR on master and it has a label patch it would release on NPM with a patch version, and so on. If it doesn't have a release label it doesn't release it (patch, minor, major). (Note: This is an idea, not required for 1.0)

That's my feedback on the 1.0 version.

[mAiNiNfEcTiOn]

(I'm now commenting separately to give my input regarding some of the points):

Regarding the contract... I think it's obvious that our main contract is the usage of the component (markup necessary + markup rendered + basic styles).

And, still on this topic, it pretty much relates to #107.

Also, IMHO, we're not in a state where we can say we're stable. I think that most of our components don't need state. Maybe some complex components would, but those are exceptions.

Besides that, what we're seeing, even internally, is that we're kind of opinionated when it comes to class names (BEM vs. non-BEM) and we're changing a lot of the components to support the className attribute (I personally think that we should change all of them to this behaviour)

/cc @Travix-International/frontend @Travix-International/frontend-epam (mentioning for full feedback)

[asci]

Thanks for your comments.

The Contract

As you said, Ricardo, the contract is a usage of component (markup, behavior, styles). But there is also a contract of each component which is basically API of this component. So, if somebody added a new component that person can expect it will work the way it was initially implemented. That person will rely on the component API. If we change its contract will be broken. That is something I want to avoid.

The Release cycle

So, in order to avoid breaking changes, I’d like to use Travix (Frint) release cycle. That is what I want to achieve. We can do it if we say that current version is stable (enough).

Regarding #107 — in the initial implementation I didn’t expect people to use inner CSS classes of elements. It was simply not in the contract of each component. And there is still no clear way how can we do allow people to modify elements appearance (and should we at all do it?). I don’t think is it a blocker for the version change.

Issues, pull requests and automated released

As I see It is not something blocking to release version 1.0. It will not change the contract of the library itself nor each component. What I can agree that is is highly desirable. But since there is no dedicated team nobody cares about something apart from components (we simply don’t have time, right?). Maybe next TIS?

BEM vs. non-BEM

The prop classNames was not in the initial contact. Personally, I don’t the idea to make UI kit so flexible, because it will definitely lead to inconsistent UI. But here I’m in the minority and will agree on what majority will decide. What will I not agree on — that we will keep mods with classNames together (and I hope you will not agree as well)? There should be only one way to achieve the thing. So, if we decide that we’re going with classNames — we can add them in release 1.0 and mark usage of mods is deprecated and remove them in version 2.0.

State-full components

I fully agree that we should have as much stateless components as possible. The thing is we shouldn’t change its contract. If we want to adopt the Travix (Frint) release cycle components should be backward compatible. And here is the questions:

How will we version each component? How to keep them backward compatible if we change their behavior?
There was one idea from Carlos. But maybe we have more?

Thanks again, hope to see your ideas on it.

[mAiNiNfEcTiOn]

Hi @asci,

Nice feedback. Here's my thoughts on it...

Where I agree

So, if somebody added a new component that person can expect it will work the way it was initially implemented. That person will rely on the component API. If we change its contract will be broken. That is something I want to avoid.

So, in order to avoid breaking changes, I’d like to use Travix (Frint) release cycle. That is what I want to achieve. We can do it if we say that current version is stable (enough).

Where I disagree

The prop classNames was not in the initial contact. Personally, I don’t the idea to make UI kit so flexible, because it will definitely lead to inconsistent UI. But here I’m in the minority and will agree on what majority will decide. What will I not agree on — that we will keep mods with classNames together (and I hope you will not agree as well)? There should be only one way to achieve the thing. So, if we decide that we’re going with classNames — we can add them in release 1.0 and mark usage of mods is deprecated and remove them in version 2.0.

I don't think we (at least you and me) are on the same page regarding className attribute.
It should be an append to the base class. I'd say it's not a breaking change, rather an extension of attributes' support. Gives the possibility to pass either CSS Modules classes (our intended case) or add custom classes.

What I don't understand:

But there is also a contract of each component which is basically API of this component.

AFAIK, the API of a component is it's "implementation markup", e.g:

<Button onClick={console.log}>Example</Button>

while markup rendered:

<button>Example</button>

(with a synthetic event listener attached to it).

Regarding styles, well I think it's reasonable to say that if you have a Sidebar panel that starts hidden and in one minor or patch release you change it to visible that will screw up the users counting with it hidden, hm?

[asci]

Hello Ricardo, thanks for ideas and thoughts!

I’ll try to elaborate a bit on topics mentioned above.

Component's API, for me, means component's Interface, which is props. The result (markup) should be considered as "grey box”, because it is platform implementation (think of React Native, for example; I’m not saying we gonna go native, but just to explain the idea — where we should put separations of concerns — markup is detail of implementation of particular component and it is private in a way)

Your example with Sidebar is perfect. Or, for example, you change the name of props onShow -> onOpen or something like this. If we didn’t catch this situation — we should pay more attention to reviews, especially on parts related to changing props and their default values (I wish we could automate it somehow)

Regarding classNames
I agree that adding className is not a breaking change. The breaking change comes from mods — there will be no need for mods if there will be classNames prop. It will confuse and will perform unnecessary code run. And moreover — there are not many places where we use mods (AFAIK only 2). But it is the different topic and we have the separate issue for that. My point here is that issue is not a blocker for the version change. Even more — would be better to resolve the issue after changing version.

Thanks

[mAiNiNfEcTiOn]

@asci agree and, in fact, we're saying the same.

The usage (JSX or React.createElement) is affected by those changes - changing the prop onShow to onOpen would break any usage with the older attribute. So, yes, we're both right and I agree with you.

Regarding the mods, yes let's leave it for the issue #107. Nevertheless, if we go for supporting both, when removing the mods support it will lead to a new major... Wouldn't be easier to remove it and fix those 2 cases you found?