search

sass / node-sass

:rainbow: Node.js bindings to libsass
https://npmjs.org/package/node-sass
MIT License
8.51k stars 1.32k forks source link

Consider use of native JS classes in place of sass_types #1088

Open saper opened 9 years ago

saper commented 9 years ago

Given that our C++ bridge can easily use existing classes to represent types, I was wondering if we should not select some existing npm modules for example for color handling (or write ours) to do the job. Writing our JS classes in C++ is crazy.

And string should be native string, null should be null.

Pity JS has no tuples, could do for colors for example.

thoughts?

xzyfer commented 9 years ago

@chriseppstein @Jakobo @davidkpiano I'd be keen to hear your thoughts on this.

saper commented 8 years ago

1310 is the first step to implement this.

xzyfer commented 8 years ago

I've given this some thought and I don't think this is a good idea, for two main reason.

Using native objects means we're completely ruling out any future opportunities to extend these types. IMHO these features are too young to know whether we'll want to do this later on.

Using native classes/primitives for some types but not other is a bad experience for developers. There is no widely available JS equivalent for a Sass Map (es6 Map aren't available on all the runtimes we support), and no JS equivalent for Sass Colors.

Instead I'd propose moving Sass types into their own npm package that node-sass plugin authors can use.

saper commented 8 years ago

Actually I like the idea of moving sass types into separate npm packages. JavaScript packages like sass-color, sass-map etc. Some existing packages for handling CSS colors could provide interface we need with ease. There is no need for any native code there.

We should provide null, true, false and a string, using V8 values.

I consider the current solution ugly and cumbersome. Maps do not behave like maps at all (https://github.com/sass/node-sass/issues/911). C++ code is hard to maintain. Providing native support for null took me like 5 minutes time (most spent on providing compatibility).

C++ sass types need to die. Really.

xzyfer commented 8 years ago

Discussion on creating a sass-types package moved to #1320.