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

support for node.js 12.x #2632

Closed
dnalborczyk opened this issue Apr 23, 2019 · 16 comments · Fixed by #2633
Closed

support for node.js 12.x #2632

dnalborczyk opened this issue Apr 23, 2019 · 16 comments · Fixed by #2633
Assignees
Labels

Comments

@dnalborczyk
Copy link

@dnalborczyk dnalborczyk commented Apr 23, 2019

node v12.0.0 just got released: https://nodejs.org/en/blog/release/v12.0.0/

installation is failing on macOS mojave

  • NPM version (npm -v): 6.9.0
  • Node version (node -v): 12.0.0
  • Node Process (node -p process.versions): [see below]
  • Node Platform (node -p process.platform): darwin
  • Node architecture (node -p process.arch): x64
  • node-sass version (node -p "require('node-sass').info"): Error: Node Sass does not yet support your current environment: OS X 64-bit with Unsupported runtime (72)
  • npm node-sass versions (npm ls node-sass): node-sass@4.11.0
// process.versions:
{
  node: '12.0.0',
  v8: '7.4.288.21-node.16',
  uv: '1.28.0',
  zlib: '1.2.11',
  brotli: '1.0.7',
  ares: '1.15.0',
  modules: '72',
  nghttp2: '1.38.0',
  napi: '4',
  llhttp: '1.1.1',
  http_parser: '2.8.0',
  openssl: '1.1.1b',
  cldr: '34.0',
  icu: '63.1',
  tz: '2018e',
  unicode: '11.0'
}
@xzyfer
Copy link
Contributor

@xzyfer xzyfer commented Apr 23, 2019

Updated. We have a PR in #2633 but are blocked on the nodejs team updating nan for Node 12.

@xzyfer
Copy link
Contributor

@xzyfer xzyfer commented Apr 24, 2019

Created a tracking issue nodejs/nan#849

@jiqiangbing
Copy link

@jiqiangbing jiqiangbing commented Apr 24, 2019

Installation and compile is failing on windows.
image
And when I change
v8::String::Utf8Value string(value);
to
v8::String::Utf8Value string(v8::Isolate::GetCurrent(), value);

It's works!
Seems String::Utf8Value with one arg is removed in new V8 of node 12.

@EricMCornelius
Copy link

@EricMCornelius EricMCornelius commented Apr 24, 2019

@xzyfer - Looks like the compilation error is originating from that unshimmed v8 string constructor, which is no longer valid. Not actually something NAN updates will fix.

You could use https://github.com/nodejs/nan/blob/master/doc/v8_misc.md#nanutf8string though.

@xzyfer
Copy link
Contributor

@xzyfer xzyfer commented Apr 25, 2019

@EricMCornelius I believe the compilation is because that nan API is using the wrong constructor.

../node_modules/nan/nan_implementation_12_inl.h:337:37: error: too few arguments to function call, expected 2, have 1
  return v8::StringObject::New(value).As<v8::StringObject>();

@EricMCornelius
Copy link

@EricMCornelius EricMCornelius commented Apr 25, 2019

@xzyfer - I get the error @jiqiangbing highlighted above at: https://github.com/sass/node-sass/blob/master/src/create_string.cpp#L17 during compilation.

Aside from that, a clean build from master is working fine on my local. Just changed line #17 to the Nan::Utf8String constructor.

@xzyfer
Copy link
Contributor

@xzyfer xzyfer commented Apr 25, 2019

@EricMCornelius even with those changes I was getting build errors locally. Looks like I had an older version on nan when I needed at least 2.13.2 - See https://github.com/nodejs/nan/blob/master/CHANGELOG.md#2132-mar-24-2019

In file included from ../src/binding.cpp:1:
In file included from ../node_modules/nan/nan.h:2657:
../node_modules/nan/nan_object_wrap.h:24:25: error: no member named 'IsNearDeath' in 'Nan::Persistent<v8::Object,
      v8::NonCopyablePersistentTraits<v8::Object> >'
    assert(persistent().IsNearDeath());

@xzyfer
Copy link
Contributor

@xzyfer xzyfer commented Apr 25, 2019

Made some tweaks to the PR and kicked off a new round of builds
See https://travis-ci.org/sass/node-sass/builds/524284615
See https://ci.appveyor.com/project/sass/node-sass/builds/24083767

@xzyfer
Copy link
Contributor

@xzyfer xzyfer commented Apr 25, 2019

CI is happy. Now we're just waiting for appveyor/ci#2921

@i5o

This comment has been minimized.

@mtimofiiv

This comment has been minimized.

@liudonghua123

This comment has been minimized.

@xzyfer
Copy link
Contributor

@xzyfer xzyfer commented Apr 26, 2019

@mtimofiiv the branch is ephemeral. Taking that advice will end up in your install breaking when the branch is deleted. Noone needs to use Node 12. The only reasonable solution is to wait.

@sass sass locked and limited conversation to collaborators Apr 26, 2019
@xzyfer
Copy link
Contributor

@xzyfer xzyfer commented Apr 26, 2019

Locked this issue since there is no need for further discussion. We know what needs to be done and we're working on it.

@xzyfer
Copy link
Contributor

@xzyfer xzyfer commented Apr 26, 2019

Re-opening because it'll take a day to build the binaries.

@xzyfer xzyfer reopened this Apr 26, 2019
nschonni referenced this issue Apr 26, 2019
@xzyfer
Copy link
Contributor

@xzyfer xzyfer commented Apr 27, 2019

Published as 4.12.0

@xzyfer xzyfer closed this as completed Apr 27, 2019
@nschonni nschonni unpinned this issue Apr 27, 2019
Tyratox added a commit to hfag/shop-frontend that referenced this issue Apr 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants