feat: v2

[scttcper]

A working build can be seen/installed at the hard fork at https://github.com/TypeCtrl/tinycolor

updated docs: https://typectrl.github.io/tinycolor/
api docs: https://typectrl.github.io/tinycolor/docs/
breaking changes: see notes

on travis this would need a few new tokens

• GH_TOKEN: is a github personal access token https://github.com/settings/tokens
• NPM_TOKEN: run npm token create used with semantic release to publish from CI. If you want to publish manually i can remove this.

notes

• reformatted into TypeScript / es2015 and requires node >= 8
  • tree shakeable "module" export and no package sideEffects
• tinycolor is now exported as a class called TinyColor
• new random, an implementation of randomColor by David Merfield that returns a TinyColor object, the old one is still available as legacyRandom
• several functions moved out of the tinycolor class and are no longer TinyColor.<function>. See updated docs for use examples.
  • readability, fromRatio moved out
  • random moved out and renamed to legacyRandom
  • toFilter has been moved out and renamed to toMsFilter
• mix, equals use the current TinyColor object as the first parameter
• added polyad colors tinycolor PR Polyad colors #126
• color wheel values (360) are allowed to over or under-spin and still return valid colors tinycolor PR Enable wrapping around of hue values given in degrees #108
• added tint() and shade() tinycolor PR Add tint() and shade() #159
• isValid, format are now properties instead of functions

closes #151

[wmertens]

This is wonderful, I just noticed that tinycolor was actually quite large, go check it out and 15 hours ago this lands :) Thanks, looking forward to the release!

[wmertens]

Oh - I mistook the closing for merging 😞

[scttcper]

@wmertens it can still be used/found at https://github.com/TypeCtrl/tinycolor however you lose the advantage of a popular repo with lots of people finding whatever bugs exist.

[wmertens]

Not much point in a popular repo where found bugs are then not actually fixed :(

@bgrins I totally understand time pressures/burnout etc, but would it not be better to get some help maintaining the repo? I (and I'm sure others) would gladly do some PR merging to assist…

[bgrins]

@scttcper Sorry for dropping the ball on this - especially after you took the time to do the updates to get this landable. I started to review it, but with the rewrite and API changes I didn't finish and it fell off my radar. If you could commit to helping out with maintaining the repo, I will get this landed.

I do have one specific question around the API methods that have moved out (random, readability, etc) - how can those be referenced from a bundle? That is, for a CDN linked JS file or one copied out through dist/ and loaded as a <script> tag rather than through a loader. I wouldn't want to lose that functionality for those users, so even if it's a bit awkward I'd prefer to continue to expose them through TinyColor.foo for backwards compat (unless if I'm missing something and they already are).

[scttcper]

@bgrins there's still a browser specific bundle created with npm run build it just isn't the "main" in package.json anymore.

if you run build and then load dist/package-dist/tinycolor.umd.min.js you should get what you would expect from a cdn

Edit: no worries btw. Open source isn't an obligation to work on a project forever 😺

[bgrins]

@bgrins there's still a browser specific bundle created with npm run build it just isn't the "main" in package.json anymore.
if you run build and then load dist/package-dist/tinycolor.umd.min.js you should get what you would expect from a cdn

OK, tested that locally with an HTML file and a script pointing to that and indeed it works just as before!

Just to confirm: the umd min module exposes an API which would require migration from:

a) tinycolor("red") to new tinycolor.TinyColor("red")
b) tinycolor.random() etc would continue to work without code changes between v1 and v2

As for (a), I have a couple of questions around backwards-compat with v1:

1. How hard would it be to support calls without new as we currently do by returning an instance if the function is called directly (i.e. https://github.com/bgrins/TinyColor/blob/master/tinycolor.js#L24-L27)? Looking at the index.ts I'm guessing that would code would still work, but maybe that kind of overloading is discouraged by TypeScript?
2. I still have a slight preference for 'tacking on' the non-instance methods onto the main object (at least for UMD) so that we don't need to export a secondary global - i.e. if the main thing we exported was TinyColor which is both a class and has some static methods like random you could do new TinyColor("red") and TinyColor.random(). This might be, along with (1), something that's not really normal practice these days and would be justified only by backwards compatibility if at all.

[bgrins]

2. I still have a slight preference for 'tacking on' the non-instance methods onto the main object

It looks like that could be handled with TypeScript static properties. We could also export the functions directly to the public API for non UMD packages (as is currently done) for convenience.

[bgrins]

One more question around backwards-compat:
3. Is the casing required / enforced by TypeScript? Currently code references through all lower-cased tinycolor so we'd be asking people to change that code to upgrade versions. Since it's more conventional to have CamelCase for classes and that's a pretty easy find/replace change, it seems like a reasonable ask if we prefer to reference it that way going forward.

[scttcper]

@bgrins i've added a default export so the following will continue to work as it does now.

const tinycolor = require('tinycolor2');
const x = tinycolor('red');
x.toHex() // "ff0000"

to use it as a class it is still

const TinyColor = require('tinycolor2').TinyColor; 
// or 
import { TinyColor } from 'tinycolor2';
const x = new TinyColor('red');
x.toHex() // "ff0000"

3. Is the casing required / enforced by TypeScript? Currently code references through all lower-cased tinycolor so we'd be asking people to change that code to upgrade versions. Since it's more conventional to have CamelCase for classes and that's a pretty easy find/replace change, it seems like a reasonable ask if we prefer to reference it that way going forward.

Can go any way you want on this one.

[bgrins]

I think I'm still seeing some issue with the way tinycolor is exported in the demo, such that tinycolor("red") in the console throws. But I'm going to merge this into a v2 branch so we can get tokens and whatnot figured out, and base additional changes on top of what you've already got.

[bgrins]

OK, now if I pull down https://github.com/bgrins/TinyColor/tree/v2 locally, here are a couple of notes:

1. npm run-script start:demo fails with

> serve demo/public
sh: serve: command not found

Maybe a dep is missing?

2. npm run build and then open the demo and it fails. I see now we need to do build:demo, but perhaps the demo should be built via build

3. Both TinyColor.random and tinycolor.random are undefined in the demo page. What do you think about adding that as per feat: v2 #169 (comment)?

[thaliaarchi]

Sweet! I've been watching this PR for a while and I'm excited to see that it made it through.

[bgrins]

Is the casing required / enforced by TypeScript? Currently code references through all lower-cased tinycolor so we'd be asking people to change that code to upgrade versions. Since it's more conventional to have CamelCase for classes and that's a pretty easy find/replace change, it seems like a reasonable ask if we prefer to reference it that way going forward.

Can go any way you want on this one.

I'd be generally inclined to export it as lower-cased so that the v1->v2 migration is smoother (and the migration docs are easier to write). But I guess it's a balance between that vs following class naming conventions and how much of a pain changing it back at this point will be.

[bgrins]

NPM_TOKEN: run npm token create used with semantic release to publish from CI. If you want to publish manually i can remove this.
GH_TOKEN: is a github personal access token https://github.com/settings/tokens

Is GH_TOKEN also only needed for npm release, or does travis need it for something else as well?

[scttcper]

GH_TOKEN is used to push a release to the github release tab w/ git tag that matches the version published on npm. example

I feel like if they're going to switch to using class with the new operator they should also switch to TinyColor since that indicates its a class. They can continue to use the tinycolor() in all lower case without changes.

on the demo, its npm run-script demo or npm run demo

[bgrins]

I feel like if they're going to switch to using class with the new operator they should also switch to TinyColor since that indicates its a class. They can continue to use the tinycolor() in all lower case without changes.

That sounds fine to me, assuming we can support the static methods on the class as mentioned in (3) in #169 (comment).

[scttcper]

@bgrins i think i understand correctly.

In node it should already be that way.

const tinycolor = require('tinycolor2')
tinycolor.random().toHex();

However its missing in the demo console. I've added it in #175