Skip to content

Everything in #34 and then color() -> color-mod() #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Everything in #34 and then color() -> color-mod() #35

wants to merge 1 commit into from

Conversation

jonathantneal
Copy link

No description provided.

@tylergaw
Copy link
Collaborator

@jonathantneal I think we should be cautious and slow to change the name to color-mod. I know folks involved in the spec; tab, chris, etc. have said things are changing, but I haven't heard them say for certain the name is changing to color-mod. Tab said specifically to ignore it for now over here #33 (comment)

Are you involved in other convos where folks have a more clear picture of what is changing in the spec and when?

@jonathantneal
Copy link
Author

Hey @tylergaw, I hope I can clear up this misunderstanding.

  1. The name color is taken. There is a spec for color(). Apple Safari has shipped color(). Continuing to use the name breaks shipped implementations.

  2. The name for the speculative function you are polyfilling is currently color-mod(). Yes, this name will probably change (as it already has).

[css-color] Rename color() to color-mod()
Temporary, bikesheddable name just to disambiguate from the new color() function
@svgeesus

The reason people are told to ignore this entire specification is because 1. nobody knows if the syntax will change, and 2. nobody knows what the new function name will be. However, we do know that the name will probably not be color(), in the same way it will probably not be var(), attr(), or linear-gradient().

All that being said, I hope you take in this new name, or at least make the name configurable, but I also respect that you don’t have to, and I understand how not changing the name reduces compatibility issues.

I’m really only hoping to avoid the necessary fork I would have to make in order to keep the PostCSS plugin to spec. Because, even if the spec completely dies, like @apply has died, the PostCSS plugin must emulate the spec as closely as possible, and should not deliberately digress from the spec.

@ai
Copy link

ai commented Oct 11, 2017

@jonathantneal we should wait for spec stabilization. @tabatkins told ignore color-mod() #33 (comment)

@tylergaw
Copy link
Collaborator

Ah, thanks for the link to the Apple color() spec! That's excellent.

Cool, just to be clear, I'm with you on making sure this project and PostCSS stay in sync with specs. Even with my off the cuff comments in #29.

I just want to make sure we don't get it too much of hurry. Let me throw out the scenario I see happening.

  • We change the name to color-mod() today
  • That name changes in the next iteration of the W3C spec.
  • We then change the name of the function again to color-thingy() and cut another major release of this lib
  • possibly repeat

Now, that may be exactly what we need to do. We're working right on the edge of this stuff so we should expect churn. And any users of this lib should expect it too. I'm still just being cautious and making sure we all now what we're setting ourselves up for.

@jonathantneal
Copy link
Author

jonathantneal commented Oct 11, 2017

@tylergaw, first, thank you for listening. I know the internet can make text cold, and I know I have very strong opinions here, so I hope I’m not coming across too rudely.

I just want to make sure we don't get it too much of hurry.

I think you may be drawing a line between now and the time you knew about this change. The name was changed in the spec once, 603 days ago, on Feb 17, 2016. This would not be a hasty change.

We're working right on the edge of this stuff so we should expect churn.

Yes, and if we do not follow these changes, or if we place a burden on the csswg to not make changes (within, say, a 603 day period), then we are actually sabotaging the future of CSS.

I know what I’ve said sounds dire, but believe me, I’ve been on the other side, I’ve ignored the warnings, and I was thinking the whole time that I was doing it with the best interests of my fellow developers in mind. But don’t take my word for it:

Process-wise, there were times when Picturefill delayed real implementations. “What’s the rush? Developers already have a solution for this in Picturefill.” So, where’s the line—what’s the hand-off? How do we build something that drives native implementations—and acts as a reference implementation itself—without disincentivizing the real thing? How do we engage browser representatives directly; how do we collaborate with browsers, solicit feedback, and—with their help—guide a script that countless developers might already be using in their day-to-day work toward acting as a reference implementation?
@Wilto, from Seeing the Extensible Web Manifesto Through

@ai
Copy link

ai commented Oct 11, 2017

@jonathantneal I really don’t think that we should change name if W3C member told us to ignore it ;)

@tabatkins
Copy link

I really don’t think that we should change name if W3C member told us to ignore it ;)

I meant ignore the feature entirely. It's not currently on a standards track at all, I just haven't taken the time to go comment it out of the draft.

If the color-modification functionality is currently named color() in this library, that's going to be a problem, since there is a standards-track function already named color() that does a different thing entirely.

@tylergaw
Copy link
Collaborator

tylergaw commented Oct 11, 2017

Howdy @tabatkins, thanks. Since we have you here...

If the color-modification functionality is currently named color() in this library...

That's correct. Ian made this lib way back in the day–like 4 years+ ago–based on your early proposed spec of modifying colors with a function named color() that was at https://drafts.csswg.org/css-color/#modifying-colors and we've made updates to keep the lib in sync with changes to that spec.

OK, we know color() is taken, so that's out completely. And as you just said color modification is not on a standards track now.

What do you think our safest, future facing change for this lib is? Some color modification feature in CSS sounds rad as hell. Think a lot of folks agree. I built this silly thing https://colorme.io some time back in anticipation of it, and using this lib.

Should we:

  • Do nothing. Don't merge this, don't touch this lib, chill
  • Change our function name to the kinda-temp name color-mod then just let the spec process simmer
  • Something else

Is it worth it for me to propose a similar question or other questions in the CSSWG mailing list or similar?

@tabatkins
Copy link

There is no safe change for this right now, if your goal is to match the future CSS spec. Anything you do will be entirely speculative, with no guarantee whatsoever that it reflects a future CSS feature. At the moment, this is entirely and completely a novel library feature, not a polyfill or prollyfill.

@jonathantneal
Copy link
Author

According to endorsed terminology, it would be a speculative polyfill, no?

speculative polyfill: Emulates a proposed feature of the web platform
Nomenclature: What is a polyfill?

@tabatkins
Copy link

Splitting hairs pretty finely at that point.

@ai
Copy link

ai commented Oct 11, 2017

If we want to support color-mod, the best plan will be:

  1. In 2.0 support both color() and color-mod().
  2. Show warning on color().
  3. Add PostCSS plugin with CLI tool to replace color()color-mod(). Mention this tool in warning.
  4. In 3.0 (after few months minimum) we drop color() support.

Dropping color() support in 2.0 will make user angry, because you just kill their code.

@tabatkins did I understand correctly, that right now W3C doesn’t plan to change color-mod name?

@tabatkins
Copy link

At the moment, the CSS spec isn't intending to support a color modification function at all. I just haven't removed it from the spec yet.

@jonathantneal
Copy link
Author

@ai, color() is a shipped feature. :( The CSSWG can’t be expected to guide us on something that’s barely lingering in the spec to begin with.

@ai
Copy link

ai commented Oct 11, 2017

@tabatkins why? color-mod can be really useful with Custom Properties.

@ai
Copy link

ai commented Oct 11, 2017

@jonathantneal I understand that color() is already shipped with Safari. Unfortunately, you will not be able to explain it to this tool users. They will be just angry and will not take care about Safari.

This is why it is very important to have migration plan and take care of users first.

Look at React migration plans.They are perfect. And they are based on the idea that every major release doesn’t break your code, only show warnings. Next major release will remove that deprecated API with warnings. but since you see warnings for few months, you already migrated to new API.

The same idea is used in PostCSS major releases.

@tabatkins
Copy link

I'm aware that color modification functions are useful - I definitely agree, which is why I wrote one into the spec in the first place! The problem is that the design is bad and needs reworking, and I haven't had time to do that.

@tylergaw
Copy link
Collaborator

Ok, thanks for diggin' in ya'll. And @tabatkins, thanks for clarifying things for us.

Here's what I'm thinking. I think @ai 's proposal is spot on and we should do that. I think we should release a 2.0.0 that supports both color and color-mod, with the warning for color that says it will 100% not be a thing in the future for this lib because it is not related to color modification. The reason it'll be a 2.0 is because we have other 1.x breaking changes in #34.

We'll also update documentation in this lib to inform folks that the future of css color modification functions is uncertain and there's a 100% chance things will change so, dragons.

Does that sound good?

@tylergaw
Copy link
Collaborator

@jonathantneal One process ask. I just went back to #34 and removed my changes to the version in package.json and the History.md.

Can you have this PR only contain your changes for the function name instead of also including the work in #34?

Then once we merge the work needed here and #34 I'll do another PR to bump the version and update the history. Want to keep the commits all tidy for posterity.

@jonathantneal
Copy link
Author

@tylergaw sure thing. Let me know if that worked or if you need anything else.

@jonathantneal jonathantneal changed the base branch from master to tg-bring-back-spaces-in-modifiers October 12, 2017 00:27
@ianstormtaylor
Copy link
Owner

+1 to the decision you all came up with!


For what it's worth, we shouldn't listen that closely to what the CSS spec authors say when they are talking about how "this isn't a polyfill" and thus shouldn't exist. It's not a polyfill for spec'd behavior, yes, that's true. But obviously they are super conservative. It is a polyfill for things that are up for consideration, and that can be useful, because otherwise the current base CSS functionality we have to work with is lacking. And we all know that waiting for things to be standardized properly in CSS land takes way too long to be useful.

The way I think of this kind of library (and the others like it) is that it is like the stage-0 and stage-1 from Babel. Except it was created before Babel, but unfortunately in CSS spec land it wasn't recognized as being a useful part of the feedback loop for the spec-writing process—unlike Babel has in JS land, which has flourished. So in that sense, we should be on the edge of what is proposed and useful, and expect breaking changes over time, just as stage-0 features can change.

(I also agree with @ai that the React migration system with deprecation warnings the release before is absolutely amazing, and a good goal to have whenever possible.)

@tabatkins
Copy link

For what it's worth, we shouldn't listen that closely to what the CSS spec authors say when they are talking about how "this isn't a polyfill" and thus shouldn't exist. It's not a polyfill for spec'd behavior, yes, that's true. But obviously they are super conservative. It is a polyfill for things that are up for consideration, and that can be useful, because otherwise the current base CSS functionality we have to work with is lacking. And we all know that waiting for things to be standardized properly in CSS land takes way too long to be useful.

This is a bad reading of what I said. This sort of stuff should definitely exist. But it's not a polyfill, because it's not trying to forward-fill any spec proposals that are expected to succeed, but haven't yet reached full implementation and penetration. Words mean things. ^_^

@tylergaw
Copy link
Collaborator

Thanks @jonathantneal that's perfect.

The remainder of work we need on this is to still allow color() as well as color-mod(). When color() is used it should console.warn Letting folks know it's Deprecated and will be removed in a future release. We can also get the deprecation notice and further docs in the readme to think to from the warning.

Do you want to continue with that work or would you rather I took it on?

@ai
Copy link

ai commented Oct 12, 2017

@tylergaw PostCSS has better warning API http://api.postcss.org/Node.html#warn

It shows origin plugin and displayed properly by PostCSS runner (for example, Atom plugin could display it inside GUI since console is hidden).

@tylergaw
Copy link
Collaborator

@ai OK, cool. Are you suggesting we copy what ya'll are doing with that or that we should use PostCSS's warning instead of logging the warning here. Or am I misunderstanding?

@ai
Copy link

ai commented Oct 12, 2017

Yeap, this warning )mostly all warnings from a plugin) must be sent by Node#warn (for better UX).

@tylergaw
Copy link
Collaborator

Hrm, I'm not sure that'll work for us here. I think this lib should log the deprecation warning because there are uses of it outside of PostCSS. We don't have anything PostCSS in the lib as of right now. Am I missing something?

@ai
Copy link

ai commented Oct 12, 2017

Oops. Sorry, I forgot that it is not postcss-color-function 😅

Maybe for postcss-color-function we can add warningBy option with optional callback?

@tylergaw
Copy link
Collaborator

Ah! haha, ok cool, no prob. So many projects! So many repos!

@tylergaw
Copy link
Collaborator

Hey @jonathantneal just checking on this again. Let me know if you want to continue with the updates above or if you want help. Either way works.

@jonathantneal
Copy link
Author

Sorry, I didn’t follow that you were waiting on me. How can I help? This is an open PR to switch to the available name. For the PostCSS plugin, we can deprecate the name however, in a major release, in a minor release. Either way, it will probably be fairly quickly subsequent releases, as to protect the color namespace.

@tylergaw
Copy link
Collaborator

Hey @jonathantneal sorry for the slow response. Yeah, I guess I'm wondering if you want to bake in the support for both color and color-mod in this PR? I'm wondering if we don't want to remove all the tests and code that handles color() right now? For example, the first change in this PR:

var index = string.indexOf('color-mod(');

We'll need to update that function to look for both right? I'm trying to determine if it's more efficient to 1) Merge this PR then circle back to re-add deprecated support for color() or 2) Update the work you have hear with support for both color() and color-mod(), plus the warning about color().

I'm a little slow to merge this into master because I don't want the change to get merged then released before we're ready. My thinking is if it's in master it should be ready to ship in a release.

What do you think?

@jonathantneal
Copy link
Author

jonathantneal commented Oct 25, 2017

@tylergaw, tricky question. If this were late 2016, I would say support both and deprecate. However, the word color was reserved about 2 years ago, and Safari shipped support for color() in March. IMO, any new release of a plugin that uses the name color would be in conflict with the existing W3C standard and implementation. Therefore, I would say release something with a deprecation notice and then immediately release the one that doesn’t use color.

If you still want to support both, you should add tests that ensure you do not transform valid uses of color() per the spec. See: https://developer.apple.com/library/content/releasenotes/General/WhatsNewInSafari/Articles/Safari_10_1.html#//apple_ref/doc/uid/TP40014305-CH12-SW16

@tylergaw
Copy link
Collaborator

Thanks @jonathantneal. We do want to support both color and color-mod. Mentioned above in #35 (comment) the rough plan.

In 2.0.0– the next release of this–we'll support and recommend color-mod and also support color. However, uses of color will receive a console.warn that we'll be deprecating that function name in 3.0.0. I'm not sure of the timeline on that, but it won't be immediate. We should give some time for folks to adjust. As mentioned in the conversation above, we're following good practices that we've seen/used from folks at React and other places.

Do you want to do the work in this PR to make this happen or do you want us to take it on?

@stevenvachon
Copy link

@tabatkins have you abandoned all of your proposals, then? You sound very pessimistic and CSS is starting to look like a piece of crap.

@tylergaw
Copy link
Collaborator

tylergaw commented Jan 22, 2018

@stevenvachon Tab gave us plenty of info to go on for this issue. We haven't made any changes yet due to other work obligations. We'll pick back up on this work in the near future. I'm sure any spec(s) around CSS color modification will also progress in time

Also what you said:

CSS is starting to look like a piece of crap

I'm a maintainer of this project and we don't do that here.

Repository owner locked as resolved and limited conversation to collaborators Jan 22, 2018
@jonathantneal jonathantneal deleted the tg-bring-back-spaces-in-modifiers branch March 30, 2019 05:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants