fix: fixed error: ‘class v8::Object’ has no member named ‘CreationCon…

[zack09holland]

…text’; allowing node-gyp to rebind with node-postal, which enables node-postal to be used on Node v20.5.0

[tomups]

@zack09holland any chance to add CI tests for this? I think it's the only thing preventing it from merging as mentioned in #43

[zack09holland]

It’s on my todo list, I just haven’t had time. With it being the holiday season, I’ll have some free time here this upcoming week. Thanks, Zachary Holland

…

On Sat, Dec 16, 2023 at 3:45 AM Tomás ***@***.***> wrote: @zack09holland <https://github.com/zack09holland> any chance to add CI tests for this? I think it's the only thing preventing it from merging as mentioned in #43 <#43> — Reply to this email directly, view it on GitHub <#44 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB7THUQWV3WZNMTUZEO2MMDYJWCXFAVCNFSM6AAAAAA3KJY4YOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJYG44TQMJTGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[tomups]

@missinglink I think the only necessary changes would be adding Node 18 and 20 in the github actions matrix?

https://github.com/openvenues/node-postal/blob/master/.github/workflows/push.yml#L12-L15

Or is there anything else you think should be tested?

It would be great to have this out as this is the only dependency preventing us from upgrading to Node 20 in our stack.

[missinglink]

adding Node 18 and 20 in the github actions matrix

Yeah exactly, could you please make that change

[tomups]

adding Node 18 and 20 in the github actions matrix

Yeah exactly, could you please make that change

@zack09holland could you make that change in the PR? I don't want to open a PR myself based on yours just for that change and take the credit 😃

[zack09holland]

Yeah, I can do that! Thanks for checking back in on this issue. Slipped my mind!

…

On Sat, Jan 20, 2024 at 3:03 PM Tomás ***@***.***> wrote: adding Node 18 and 20 in the github actions matrix Yeah exactly, could you please make that change @zack09holland <https://github.com/zack09holland> could you make that change in the PR? I don't want to open a PR myself based on yours just for that change and take the credit 😃 — Reply to this email directly, view it on GitHub <#44 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB7THUVV6QW2QBOHPF5UGJLYPQWJBAVCNFSM6AAAAAA3KJY4YOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBSGI3DINJRHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[zack09holland]

@tomtastico @missinglink Pushed the requested changes! Let me know if there is anything else.

[missinglink]

Unfortunately the tests aren't running on GitHub and I'm in New Zealand without a laptop.

Sorry to be a pain but could you please update the GitHub workflow so it triggers on pull requests too, I believe the correct syntax is:

on: [ push, pull_request ]

ref: https://docs.github.com/en/enterprise-cloud@latest/actions/using-workflows/triggering-a-workflow#using-multiple-events

If you can't get it running I'll sort it out once I'm back in Berlin next week.

[zack09holland]

Nice catch. Looks to be running now with my latest push.

[zack09holland]

@tomtastico @missinglink Alright I may need a second set of eyes to figure out how to get all the tests to pass...

I did have it working where tests for node 16, 18, and 20 passed. Tried to do some further testing to get v12 and v14 to work but it kept failing. So I decided to revert back but when even reverting the changes back to what had passed, everything continued to fail...

Another thing to note, even with this new/change fix in place, node v12 and v14 I believe will continue to fail. Would seem that this version of node-postal and the fork created by @cymen (https://github.com/cymen/node-postal) are quite different. His version seems to work for node v12 and v14 where as this one version I had difficulties getting it to work?

[missinglink]

Not sure what's going on, I opened a PR to trigger the CI against the latest master branch and it passed #48

I can't really do much more from my phone but I'll have another look next week.

[albarrentine]

Maybe try upgrading Nan? That handles many of the changes in the C++ APIs across versions. From searching around it seems the issue with CreationContext in Node 20 is resolved with 2.17.0 (https://www.npmjs.com/package/nan/v/2.17.0). We're currently on 2.14.0

[zack09holland]

@albarrentine @missinglink @tomtastico Fresh set of eyes really does wonders. Was able to get all tests to pass finally!

[missinglink]

Amazing, thanks so much 🎉

[missinglink]

Seems that publishing a new version to npm requires credentials, unfortunately I won't be able to do that until next week.

Until then the master branch can be used for testing as so: npm install https://github.com/openvenues/node-postal

[missinglink]

Version 1.2.0 is now published on npm.