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

iD fails to build on Node 10 #5028

Closed
greenkeeper bot opened this issue May 7, 2018 · 4 comments
Closed

iD fails to build on Node 10 #5028

greenkeeper bot opened this issue May 7, 2018 · 4 comments
Labels
chore-build Improvements to the iD build scripts / CI environment

Comments

@greenkeeper
Copy link
Contributor

greenkeeper bot commented May 7, 2018

Version 10 of Node.js (code name Dubnium) has been released! 🎊

To see what happens to your code in Node.js 10, Greenkeeper has created a branch with the following changes:

  • Added the new Node.js version to your .travis.yml
  • The new Node.js version is in-range for the engines in 1 of your package.json files, so that was left alone

If you’re interested in upgrading this repo to Node.js 10, you can open a PR with these changes. Please note that this issue is just intended as a friendly reminder and the PR as a possible starting point for getting your code running on Node.js 10.

More information on this issue

Greenkeeper has checked the engines key in any package.json file, the .nvmrc file, and the .travis.yml file, if present.

  • engines was only updated if it defined a single version, not a range.
  • .nvmrc was updated to Node.js 10
  • .travis.yml was only changed if there was a root-level node_js that didn’t already include Node.js 10, such as node or lts/*. In this case, the new version was appended to the list. We didn’t touch job or matrix configurations because these tend to be quite specific and complex, and it’s difficult to infer what the intentions were.

For many simpler .travis.yml configurations, this PR should suffice as-is, but depending on what you’re doing it may require additional work or may not be applicable at all. We’re also aware that you may have good reasons to not update to Node.js 10, which is why this was sent as an issue and not a pull request. Feel free to delete it without comment, I’m a humble robot and won’t feel rejected 🤖


FAQ and help

There is a collection of frequently asked questions. If those don’t help, you can always ask the humans behind Greenkeeper.


Your Greenkeeper Bot 🌴

@bhousel
Copy link
Member

bhousel commented May 8, 2018

ugh, our build fails on Node 10. I'll need to dig into this more this week. 😞
The issue has something to do with how the build_data.js script is using node fs library.

node[76949]: ../src/node_file.cc:829:void node::fs::Stat(const FunctionCallbackInfo<v8::Value> &): Assertion `(argc) == (3)' failed.
 1: node::Abort() [/Users/bryan/.nvm/versions/node/v10.0.0/bin/node]
 2: node::InternalCallbackScope::~InternalCallbackScope() [/Users/bryan/.nvm/versions/node/v10.0.0/bin/node]
 3: node::fs::LStat(v8::FunctionCallbackInfo<v8::Value> const&) [/Users/bryan/.nvm/versions/node/v10.0.0/bin/node]
 4: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo*) [/Users/bryan/.nvm/versions/node/v10.0.0/bin/node]
 5: v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) [/Users/bryan/.nvm/versions/node/v10.0.0/bin/node]
 6: v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) [/Users/bryan/.nvm/versions/node/v10.0.0/bin/node]
 7: 0x285a870427d

If I comment out build_data.js, no errors (though obviously no build either):

var buildData = require('./build_data')(isDevelopment);

@bhousel bhousel reopened this May 8, 2018
@bhousel bhousel added chore-build Improvements to the iD build scripts / CI environment and removed chore-greenkeeper labels May 8, 2018
@bhousel bhousel changed the title Version 10 of node.js has been released iD fails to build on Node 10 May 8, 2018
@mmd-osm
Copy link
Contributor

mmd-osm commented May 10, 2018

You're not alone, this one is somewhat similar: GoogleChrome/workbox#1450

@bhousel
Copy link
Member

bhousel commented May 10, 2018

In the meantime I can probably change to a simpler version of the build file without the Promises and callbacks. It's really just concatenating things together. (update: it wasn't this at all)

@bhousel
Copy link
Member

bhousel commented May 11, 2018

It turned out to be an issue with the older version of std/esm, which we are using to let us require ESM lodash in our build scripts.

We can upgrade std/esm (now just called esm) to the current version now that we've dropped support for Node 4 (see: a065f2f)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore-build Improvements to the iD build scripts / CI environment
Projects
None yet
Development

No branches or pull requests

2 participants