cssinjs / jss

JSS is an authoring tool for CSS which uses JavaScript as a host language.
https://cssinjs.org
MIT License
7.06k stars 397 forks source link

[jss-observable] [jss-function] Move function values and observable values to a plugin in a separate repository #700

Closed kof closed 5 years ago

kof commented 6 years ago

I think we should move them, because too many people are not using them, so it would reduce bundle size for them. It will still stay in jss-preset-default, so for all people who use them, it won't be a change.

Todo

kof commented 5 years ago

@cssinjs/core I am gonna do this now, since we have mono repo and separating packages is cheap. Biggest benefit is bundle size reduction for those who don't use functional dsl nor observables for e.g. mui cc @oliviertassinari

kof commented 5 years ago

Also @cssinjs/core do you think jss-observable and jss-function are good names?

HenriBeck commented 5 years ago

I think jss-observable is fine. Don't know about jss-function though, as it's a bit misleading in my opinion. What about jss-function-values?

kof commented 5 years ago

I started to extract jss-observable, but realized many things are not ready where @HenriBeck is working on.

Mb its better for @HenriBeck to take over this one and start with these 2 plugins, before continuing with the other since we have an important benefit from doing these 2 first.

oliviertassinari commented 5 years ago

Can't wait to see this story completed :)!

HenriBeck commented 5 years ago

will work on it after I resolved the issues with rollup in #748

kof commented 5 years ago

I don' like jss-function name either, butthe thing is it is both, fn rules and fn values. Mb jss-function-value-and-rule

HenriBeck commented 5 years ago

Hmmm... yeah. Maybe jss-function-plugin and jss-observable-plugin?

kof commented 5 years ago

That would be inconsistent and still doesn't tell that its about the jss syntax

On Mon, Jul 9, 2018, 13:04 Henri notifications@github.com wrote:

Hmmm... yeah. Maybe jss-function-plugin and jss-observable-plugin?

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/cssinjs/jss/issues/700#issuecomment-403442035, or mute the thread https://github.com/notifications/unsubscribe-auth/AADOWClNXKzdPPX0ClVf0MdSC_QYp9_Kks5uEzijgaJpZM4TUlKD .

kof commented 5 years ago

What about jss-function-syntax ? Still a bit inconsistant with other syntactic plugins, but at least less confusing regarding "plugin"

I made a mistake of not prefixing all plugins with "plugin" eg jss-plugin-xxx ... mb its time to change that.

On Mon, Jul 9, 2018, 13:04 Henri notifications@github.com wrote:

Hmmm... yeah. Maybe jss-function-plugin and jss-observable-plugin?

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/cssinjs/jss/issues/700#issuecomment-403442035, or mute the thread https://github.com/notifications/unsubscribe-auth/AADOWClNXKzdPPX0ClVf0MdSC_QYp9_Kks5uEzijgaJpZM4TUlKD .

HenriBeck commented 5 years ago

As the observable plugin has been moved out of jss, I will start working on the function rules plugin.

jss-function-syntax seems to be a good name. Then jss-observable-syntax aswell?

kof commented 5 years ago

I checked babel naming convention, they have this: babel-plugin-syntax-{something}, I think we should start doing this for syntactic stuff

kof commented 5 years ago

jss-plugin-syntax-function and jss-plugin-syntax-observable

kof commented 5 years ago

wait, since we wanted to have functions as a value in some other cases as well, for e.g. jss-compose and jss-extend, this will become even more unclear, since functions in those are not part of this plugin.

I think we have to become even more explicit: "jss-plugin-syntax-rule-value-function" For e.g. see "babel-plugin-syntax-object-rest-spread"

HenriBeck commented 5 years ago

Okay, so we change the names of the other plugins as well then?

kof commented 5 years ago

Yeah, I think we should do this now, since we are making a big change anyways.

kof commented 5 years ago

Dunno if this means we have to increase a major version, I think we don't since its a new package and we can start it whereever we want. Api doesn't change here, so it seems we have no breaking change.

kof commented 5 years ago

and jss-plugin-syntax-rule-value-observable

HenriBeck commented 5 years ago

I think we should because we definitely need to increase the major version of jss. And having all of the packages on the same version at all times is a good idea, I think.

Will change the observable package name then.

kof commented 5 years ago

Why would you need to increase major of jss, do we have any breaking changes?

HenriBeck commented 5 years ago

Yeah, removing the function and observable plugin?

kof commented 5 years ago

Oh, sorry, I forgot, we moved away those plugins, it is definitely breaking change for the core.

HenriBeck commented 5 years ago

So we could just publish all of the plugins with version 10.0.0

kof commented 5 years ago

yep

kof commented 5 years ago

Should we close this one?

HenriBeck commented 5 years ago

yes, I think we can close this