Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

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

Open
saper opened this issue Aug 19, 2015 · 5 comments
Open

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

saper opened this issue Aug 19, 2015 · 5 comments

Comments

@saper
Copy link
Member

saper commented Aug 19, 2015

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
Copy link
Contributor

xzyfer commented Sep 9, 2015

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

@saper
Copy link
Member Author

saper commented Dec 20, 2015

#1310 is the first step to implement this.

@xzyfer
Copy link
Contributor

xzyfer commented Dec 22, 2015

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
Copy link
Member Author

saper commented Dec 22, 2015

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 (#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
Copy link
Contributor

xzyfer commented Dec 27, 2015

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants