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

[WIP][RFC] Introduce JavaScript null value #1310

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

saper
Copy link
Member

@saper saper commented Dec 20, 2015

I believe that having Sass.NULL (and Sass.TRUE,
Sass.FALSE) is incorrect. We should be using
real JavaScript primitives instead, if only
to avoid object identity issues, or to allow
natural constructs like:

done(a === b)

I suggest assigning JavaScript 'null' to
sass.NULL and sass.types.Null.NULL names.

Current stage:

  • handling primitive null values
  • handling null items in list
  • handling null keys and values in maps
  • "a instanceof sass.types.Null" does not work

An attempt to instantiate sass.NULL object via:

 new sass.types.Null()

gives a different error message now:

 TypeError: sass.types.Null is not a function

Whenever done() is called we should check for the sass.NULL
value as well.

While here, allow calling done(null) to anticipate
removal of the sass.NULL one day.

Fixes sass#1296
When the test fails, we are getting actual
values in the error message instead of

+ true
= false
If we implement sass.types.Null as an Object it gives
a different error message.
@saper
Copy link
Member Author

saper commented Dec 26, 2015

I don't think that "null instanceof sass.types.Null" could be made true, I'd rather modify our test instead (not to depend on the implementation internals).

@xzyfer
Copy link
Contributor

xzyfer commented Dec 27, 2015

I'm not convinced this is a good idea discussed briefly in #1088 (comment)

I would hold off on this until we reach a consensus on #1320

@saper saper force-pushed the master branch 2 times, most recently from 6c128d9 to 1e4bba8 Compare April 19, 2016 21:45
jiongle1 pushed a commit to scantist-ossops-m2/node-sass that referenced this pull request Apr 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants