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 support for node 18 by updating node-sass #13493

Merged
merged 1 commit into from
Feb 7, 2023

Conversation

852Kerfunkle
Copy link
Contributor

And drop support for node 12.

It's probably time, it's the next LTS :)

I have not fully tested it (too reliant on CI maybe), but it works in dev. It's possible it'll also work with node 19, but I did not try.

@bjsplat
Copy link
Collaborator

bjsplat commented Feb 1, 2023

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Feb 1, 2023

Reviewer - this PR has made changes to one or more package.json files.

@bjsplat
Copy link
Collaborator

bjsplat commented Feb 1, 2023

@sebavan
Copy link
Member

sebavan commented Feb 1, 2023

And drop support for node 12.

It's probably time, it's the next LTS :)

I have not fully tested it (too reliant on CI maybe), but it works in dev. It's possible it'll also work with node 19, but I did not try.

Was this checked on the forum before ? We can not ship this without an extensive set of tests and compatibility assessment from some of our existing dependencies.

I will let @RaananW check tomorrow.

@sebavan sebavan requested a review from RaananW February 1, 2023 22:35
@852Kerfunkle
Copy link
Contributor Author

And drop support for node 12.
It's probably time, it's the next LTS :)
I have not fully tested it (too reliant on CI maybe), but it works in dev. It's possible it'll also work with node 19, but I did not try.

Was this checked on the forum before ? We can not ship this without an extensive set of tests and compatibility assessment from some of our existing dependencies.

I will let @RaananW check tomorrow.

Fair enough. I needed it, so I PR'd it. It'll have to happen eventually, feel free to treat it as a draft or to reject it.

Cheers.

@sebavan
Copy link
Member

sebavan commented Feb 1, 2023

And drop support for node 12.
It's probably time, it's the next LTS :)
I have not fully tested it (too reliant on CI maybe), but it works in dev. It's possible it'll also work with node 19, but I did not try.

Was this checked on the forum before ? We can not ship this without an extensive set of tests and compatibility assessment from some of our existing dependencies.
I will let @RaananW check tomorrow.

Fair enough. I needed it, so I PR'd it. It'll have to happen eventually, feel free to treat it as a draft or to reject it.

Cheers.

It will for sure happen soon knowing @RaananW, I just want to be sure he can validate all the dependencies before merging it.

@852Kerfunkle
Copy link
Contributor Author

@sebavan Oh by the way on the note of

Was this checked on the forum before ?

There isn't really a "Development" section on the forum. Either that or I'm not invited :D . It would maybe be useful.

@sebavan
Copy link
Member

sebavan commented Feb 1, 2023

There isn't really a "Development" section on the forum. Either that or I'm not invited :D . It would maybe be useful.

You are correct, I think those conversations usually end up in either "feature requests" or "questions".

cc @carolhmj our forum GURU

@852Kerfunkle
Copy link
Contributor Author

There isn't really a "Development" section on the forum. Either that or I'm not invited :D . It would maybe be useful.

You are correct, I think those conversations usually end up in either "feature requests" or "questions".

cc @carolhmj our forum GURU

Indeed, I also just realised "feature requests" doubles for this stuff. No worries, I'm happy knowing this now!

@carolhmj
Copy link
Contributor

carolhmj commented Feb 2, 2023

Yup Feature Requests works fine for that :)

@RaananW
Copy link
Member

RaananW commented Feb 3, 2023

Thanks so much for the PR.

I would rather test before making changes. I agree - we should fully support node 18. But I am not 100% sure this is the only change.
I'm moving to to draft for now, until I get a change to fully test that

@RaananW RaananW marked this pull request as draft February 3, 2023 11:23
@RaananW RaananW marked this pull request as ready for review February 7, 2023 17:28
@RaananW RaananW merged commit fe9f13b into BabylonJS:master Feb 7, 2023
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.

5 participants