Skip to content
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

v8 8.4 in Node 14.6+ changed the toString behaviour for sass.types #2972

Open
nschonni opened this issue Sep 27, 2020 · 5 comments
Open

v8 8.4 in Node 14.6+ changed the toString behaviour for sass.types #2972

nschonni opened this issue Sep 27, 2020 · 5 comments

Comments

@nschonni
Copy link
Contributor

nschonni commented Sep 27, 2020

  • NPM version (npm -v): 6.14.8
  • Node version (node -v): 14.12.0, but anything above 14.5.0 has this
  • Node Process (node -p process.versions):
{
  node: '14.12.0',
  v8: '8.4.371.19-node.16',
  uv: '1.39.0',
  zlib: '1.2.11',
  brotli: '1.0.9',
  ares: '1.16.0',
  modules: '83',
  nghttp2: '1.41.0',       
  napi: '7',
  llhttp: '2.1.2',
  openssl: '1.1.1g',       
  cldr: '37.0',
  icu: '67.1',
  tz: '2020a',
  unicode: '13.0'
}
  • Node Platform (node -p process.platform): win32, but all in CI as well
  • Node architecture (node -p process.arch): x64, but all in CI as well
  • node-sass version (node -p "require('node-sass').info"):
  • npm node-sass versions (npm ls node-sass): 4.14.1

Upstream node issue nodejs/node#35365 was closed as a wont-fix, as the behaviour is a change in v8.
The solution seems to be set the @@toStringTag or https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol/toStringTag

@nschonni nschonni changed the title v8 8.4 in Node 14.6+ changed the toString behaviour v8 8.4 in Node 14.6+ changed the toString behaviour for sass.types Sep 27, 2020
@xzyfer
Copy link
Contributor

xzyfer commented Sep 27, 2020 via email

@nschonni
Copy link
Contributor Author

It still runs, but the the toString in the following now just returns [object Object], rather than [object SassColor] like it was previously, which is way CI was failing on 14.6 and above

      var t = sass.types.Color();
      assert.equal(t.toString(), '[object SassColor]');

I don't think this makes a real functionally difference for the runtime, except maybe some extensions or advanced usage.

If we can't figure out the fix, then the tests could possibly be disabled, but I think trying to find a fix would be a good idea first

@xzyfer
Copy link
Contributor

xzyfer commented Sep 27, 2020 via email

nschonni added a commit to nschonni/node-sass that referenced this issue Sep 28, 2020
nschonni added a commit that referenced this issue Sep 28, 2020
@nschonni
Copy link
Contributor Author

Got a hint over on the node issue that lead me over to here https://hyperandroid.com/2020/02/12/javascript-native-wrappers-in-v8-part-i/
But I kind of hit a wall trying to understand v8::Isolate, which is one of the parameters of the v8::Symbol::GetToStringTag(isolate) call

@hyperandroid
Copy link

hyperandroid commented Dec 9, 2020

This call v8::Symbol::GetToStringTag(isolate) returns a Symbol object, which is how v8 will resolve the prototype object name. There's not much to it, this is how Symbols are built natively.
As you say, the runtime is not very concerned on object's toStringTag, it is more a human-side thing.
This native code is que equivalent of calling this in js (maybe a quick fix could be adding these calls in js for each object needing it):

Object.defineProperty(
  obj, 
  Symbol.toStringTag, { 
    configurable: true,
    value: ‘MyObj’
  });

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

No branches or pull requests

3 participants