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

[flatbuffers] Fix CI in node 20 #1197

Merged
merged 1 commit into from
Oct 26, 2023
Merged

Conversation

ibc
Copy link
Member

@ibc ibc commented Oct 26, 2023

@ibc
Copy link
Member Author

ibc commented Oct 26, 2023

Next in a new PR I'm gonna remove mediasoup.version getter. It's a complete hack (in involves fs.writeFileSync() in JS transpiled files in node/lib folder). I know this is a breaking change since mediasoup.version is a public getter but I think we can live with it. @nazar-pc @jmillan thoughts?

@ibc
Copy link
Member Author

ibc commented Oct 26, 2023

So @nazar-pc suggest making mediasoup.version getter run fs.readfileSync(path.join(__dirname__, '..', 'package.json') and parse it and return the value of version field. It's an overkill IMHO.

@nazar-pc
Copy link
Collaborator

It is not a getter, it is a constant, so you'll need to do that in (() => {})() so it is called immediately.

This is a public API you've committed to, breaking it isn't nice. I'm not sure why people would use that field, but maybe a bit less bad alternative than removing it would be to fix it at soem value like current release and mark as deprecated for people to not rely on anymore.

@ibc
Copy link
Member Author

ibc commented Oct 26, 2023

I'll check keep it as it is. The issue is fixed in this PR so I won't complicate my life more than it is already. In v4 this will be removed.

@ibc ibc merged commit 8e605ae into flatbuffers Oct 26, 2023
32 checks passed
@ibc ibc deleted the flatbuffers-fix-ci-node-20 branch October 26, 2023 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants