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

Add node 13-17 support, bump to v0.21.0 #825

Merged
merged 56 commits into from
Jan 3, 2022
Merged

Add node 13-17 support, bump to v0.21.0 #825

merged 56 commits into from
Jan 3, 2022

Conversation

wayneparrott
Copy link
Collaborator

Public API Changes
none

Description

  1. package.json
  • Bumped to version 0.21.0
  • Support for node 13-16 required forking and customizing the following
    packages:
    @rclnodejs/ref-array-di
    @rclnodejs/ref-napi
    @rclnodejs/ref-struct-di
    Revised package dependencies to use the 3 packaged listed above
  • Added jsdoc as devDependency
  1. test-security-related.js
  • Fixed minor assertion error caused by change in Nodejs error message.
  1. Docs
  • Updated README.md, Build.md and npmjs-readme.md to expand valid
    versions of node 13-16.
  • Modified docs/Makefile to use npx to run jsdoc to generate api
    documentation

Fix #735 #717

minggangw and others added 6 commits September 27, 2021 15:51
0. package.json
* Bumped to version 0.21.0
* Support for node 13-16 required forking and customizing the following
packages:
@rclnodejs/ref-array-di
@rclnodejs/ref-napi
@rclnodejs/ref-struct-di
Revised package dependencies to use the 3 packaged listed above
* Added jsdoc as devDependency

2. test-security-related.js
Fixed minor assertion error caused by change in Nodejs error message.

3. Docs
* Updated README.md, Build.md and npmjs-readme.md to expand valid
versions of node 13-16.
* Modified docs/Makefile to use `npx` to run jsdoc to generate api
documentation
@coveralls
Copy link

coveralls commented Dec 22, 2021

Coverage Status

Coverage decreased (-0.07%) to 90.471% when pulling 4ce69c6 on wayneparrott:develop into 5f84174 on RobotWebTools:develop.

@wayneparrott
Copy link
Collaborator Author

On linux running install with node16 as root is a PIA with permissions issues. No such issues with node 12 & 14. Thus the zillion commits as I slowly convert from running scripts and test suite to non-root user. Grant me patience!

@wayneparrott wayneparrott changed the title Add node 13-16 support, bump to v0.21.0 Add node 13-17 support, bump to v0.21.0 Jan 3, 2022
@wayneparrott wayneparrott merged commit 079492c into RobotWebTools:develop Jan 3, 2022
wayneparrott added a commit that referenced this pull request Jan 3, 2022
Updated for use with node v13-v17, bump to v0.21.0 (#825)

0. package.json
* Bumped to version 0.21.0
* Support for node 13-17 required forking and customizing the following
packages:
@rclnodejs/ref-array-di
@rclnodejs/ref-napi
@rclnodejs/ref-struct-di
Revised package dependencies to use the 3 packaged listed above
* Added jsdoc as devDependency
* Moved dtslint pkg to devDependents

2. test-security-related.js
Fixed minor assertion error caused by change in Nodejs error message.

3. Docs
* Updated README.md, Build.md and npmjs-readme.md to expand valid
versions of node 13-17.
* Modified docs/Makefile to use `npx` to run jsdoc to generate api
documentation

4. Circleci - refactored config.yml to use nvm 0.39 and run test suite on node 12, 14, 16, 17

5. appveyor.yml - use nvm 0.39 and run test suite on node 12, 14, 16, 17

6. .travis.yml - use nvm 0.39 and run test suite on node 12, 14, 16, 17

7.  Fix cpplint error for node > 12 in rcl_bindings.cpp
@felixdivo
Copy link
Contributor

In the very beginning of README.md, the node tag still mentions the old version restriction.

@wayneparrott
Copy link
Collaborator Author

@felixdivo hi, the node tag is dynamically generated from https://img.shields.io/node/v/rclnodejs. When we publish the next rclnodejs release to npmjs.com the tag should be updated with the latest range of supported node versions.

@felixdivo
Copy link
Contributor

Ah okay, sorry for the bump.
Neat! 😃

@minggangw
Copy link
Member

There is an update, saying that this crash issue (from node v13 to v16) has been fixed on node v17, but with performance regression. @wayneparrott

@minggangw
Copy link
Member

I verify rclnodejs v0.20.1 with node v17.4.0 and the crash is gone.

@wayneparrott
Copy link
Collaborator Author

I verify rclnodejs v0.20.1 with node v17.4.0 and the crash is gone

Was this build using the original ref-napi and related packages or did you use the forked and udpated @rclnodejs/ref-napi and related packages?

@Zaaler
Copy link

Zaaler commented Apr 5, 2022

I'm running rclnodejs v0.21.0 with node v17.8.0 on aarch64 and ros2 galactic and I'm experiencing the cannot reinterpret from nullptr pointer.

I found a work around earlier but, I've since flashed the computer and now, I'm unfortunately debugging something I already solved. I think I may have reverted node to 16 and used the previously mentioned solution.

Should I be experiencing this error still?

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

Successfully merging this pull request may close these issues.

Compilation warning on newer node versions: v8 has a new ArrayBuffer BackingStore API
5 participants