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

KTX2Loader: Update ktx-parse dependency to v0.7.1 #29060

Merged
merged 1 commit into from
Sep 15, 2024

Conversation

yue4u
Copy link
Contributor

@yue4u yue4u commented Aug 4, 2024

Related issue: donmccurdy/KTX-Parse#147

Description

Update ktx-parse dependency from v0.3.1 to v0.7.1 (donmccurdy/KTX-Parse@v0.3.1...v0.7.1) to contain the fix in donmccurdy/KTX-Parse#148.

I couldn't find some good instructions about how to update this compressed module so I did this PR by guessing. Please tell me if some of the processes are wrong.

The steps I took:

  1. Clone https://github.com/donmccurdy/KTX-Parse
  2. Check out the release tag: git fetch --all && git checkout v0.7.1
  3. Remove --no-compress in package.json
  4. Run yarn && yarn dist
  5. Copy and paste from dist/ktx-parse.esm.js to examples/jsm/libs/ktx-parse.module.js
  6. Remove //# sourceMappingURL=ktx-parse.esm.js.map

@Mugen87 Mugen87 requested a review from donmccurdy August 5, 2024 18:22
@donmccurdy
Copy link
Collaborator

donmccurdy commented Aug 7, 2024

The steps listed do sound correct, but I'm away from my computer for a couple weeks and unable to test in the meantime. If others have tested and this works, feel free to merge!

Alternatively the ESM build from npm can be used, after removing the sourcemap line. Published builds of ktx-parse are not minified.

Copy link
Collaborator

@donmccurdy donmccurdy left a comment

Choose a reason for hiding this comment

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

Thank you!

@donmccurdy donmccurdy merged commit f509e7b into mrdoob:dev Sep 15, 2024
11 checks passed
@Mugen87 Mugen87 added this to the r169 milestone Sep 15, 2024
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.

3 participants