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

chore: update node to 16 #781

Merged
merged 1 commit into from Jan 19, 2022
Merged

chore: update node to 16 #781

merged 1 commit into from Jan 19, 2022

Conversation

birkskyum
Copy link
Member

@birkskyum birkskyum commented Jan 15, 2022

The current Node LTS is 16, bundling the npm v8 which use package-lock.json v2 format.

Update node to 16

@wipfli
Copy link
Member

wipfli commented Jan 15, 2022

Thanks for taking the time to look into this. Currently, we recommend using nvm use 14, which keeps the problems with package lockfiles out of the way. Does nvm use 16 work with this codebase?

@wipfli
Copy link
Member

wipfli commented Jan 15, 2022

If you update node and npm, please also edit https://github.com/maplibre/maplibre-gl-js/blob/main/CONTRIBUTING.md

@github-actions
Copy link
Contributor

github-actions bot commented Jan 15, 2022

Bundle size report:

Size Change: 0 B
Total Size Before: 194 kB
Total Size After: 194 kB

Output file Before After Change
maplibre-gl.js 185 kB 185 kB 0 B
maplibre-gl.css 9.49 kB 9.49 kB 0 B
ℹ️ View Details No major changes

@birkskyum
Copy link
Member Author

birkskyum commented Jan 15, 2022

The upstream node-canvas package supposedly have node 16 support since 2.8, but there appears to be some issues. When the upsteam is resolved, I'll try to run the maplibre-gl-js tests again.

@birkskyum birkskyum changed the title chore: update package-lock.json to v2 format chore: update node to 16 Jan 15, 2022
@birkskyum birkskyum force-pushed the update-lockfile branch 2 times, most recently from e21c931 to 232e392 Compare January 19, 2022 09:12
@birkskyum
Copy link
Member Author

birkskyum commented Jan 19, 2022

@wipfli I bumped node-canvas and gl because their newer versions does have node 16 support. Can we rerun the tests?

@birkskyum
Copy link
Member Author

Awesome! Let me know if anything is holding this back.

@HarelM
Copy link
Member

HarelM commented Jan 19, 2022

I think the only thing holding this back is my hesitation to upgrade my dev env to node 16... :-(
Other than that, I think we should move forward. :-)

@birkskyum
Copy link
Member Author

birkskyum commented Jan 19, 2022

I think the only thing holding this back is my hesitation to upgrade my dev env to node 16... :-( Other than that, I think we should move forward. :-)

Great :) nvm to the rescue. I'll rebase the affected pr's to this branch then.

@birkskyum
Copy link
Member Author

@wipfli , @astridx , @HarelM - I love what you have done to this repo since the fork - setting up a pipeline, typescript, jest etc. It's a big project, so if you can use an extra set of hands, I'd be happy to chip in.

@HarelM
Copy link
Member

HarelM commented Jan 19, 2022

We'd appreciate any extra hands :-)
Let us know if you would like to be added as a maintainer.

@birkskyum
Copy link
Member Author

We'd appreciate any extra hands :-) Let us know if you would like to be added as a maintainer.

Alright, that would be great actually :)

@HarelM
Copy link
Member

HarelM commented Jan 19, 2022

Added as a maintainer. Godspeed!
Feel free to ping me in maplibre slack if you need to say something in private.
I'm pushing the button... :-)

@HarelM HarelM merged commit 86f1b64 into maplibre:main Jan 19, 2022
@wipfli
Copy link
Member

wipfli commented Jan 20, 2022

Welcome to MapLibre, Birk!

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.

None yet

3 participants