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

Remove jose dependency, fix readme example #103

Merged
merged 1 commit into from
Jun 21, 2024
Merged

Remove jose dependency, fix readme example #103

merged 1 commit into from
Jun 21, 2024

Conversation

bluefoxy009
Copy link
Contributor

The current implementation of XboxTokenManager uses the exportJWK method of jose. Internally, jose ensures the type of the provided key to be KeyObject (available since Node v11.6.0 and in Bun v1.0.5) and returns the value of it's export method. This adds overhead and causes compatibility issues (see #95). By directly calling the export method of the KeyObject in the constructor, we remove the need for jose and achieve compatibility with multiple runtimes.

The second part of the pull request fixes the example in the README, where getMinecraftJavaToken doesn't automatically fetch the profile. The example has been updated to include the option fetchProfile: true so the expected response is accurate to what would actually be returned by the example.

This PR closes #88 and #95

Copy link

Removed dependencies detected. Learn more about Socket for GitHub ↗︎

🚮 Removed packages: npm/[email protected]

View full report↗︎

Copy link
Member

@extremeheat extremeheat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JWK export method if I recall was only added into in node 18 by the creator of Jose. But it seems that since we’ve recently updated the CI to v20 it indeed would make sense to use the standard method here.

@bluefoxy009
Copy link
Contributor Author

i don't know who added it but node's docs show v15.6.0, but reguardless it's implemented so good to use it

image

@extremeheat
Copy link
Member

18 came shortly after 16 because of a security issue. When the code was written we were on 14. My comment was that we did discuss using new 16/18 methods but we didn’t want to break compat by requiring 18 as mineflayer and all the other PrismarineJS were still supporting >=v14 of node. But yes this shouldn’t be a problem anymore.

@extremeheat
Copy link
Member

Thanks for looking into this, LGTM.

@extremeheat extremeheat merged commit e70f42e into PrismarineJS:master Jun 21, 2024
3 checks passed
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.

2 participants