Skip to content

fix(sandpack): accurately resolve GitHub versions in jsdelivr fetcher #7133

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

Conversation

wiledal
Copy link
Contributor

@wiledal wiledal commented Nov 10, 2022

What kind of change does this PR introduce?

Bug fix 🐛

What is the current behavior?

Old method did not accurately resolve jsdelivr urls in package.json for GitHub repos with a supplied version (eg username/repo#1.2.3 or username/repo@branch). Hard coded master meta fetching made the versioning obsolete, and repos without a master branch didn't work at all.

Issue can be seen here https://codesandbox.io/s/r3-rapier-branding-test-8e257y?file=/package.json
Screenshot 2022-11-10 at 23 08 44

More info an digging here:
#6514 (comment)

What is the new behavior?

This update makes it possible to accurately pull specific release versions from GitHub repos, and the default branch is fetched from the repository meta instead of defaulting to master.

What steps did you take to test this? This is required before we can merge, make sure to test the flow you've updated.

  • Ran the application locally, confirmed working as expected
  • Ran yarn test, all tests are still passing, nothing changed here

Checklist

  • Documentation (comments in code)
  • Testing
  • Ready to be merged
  • Added myself to contributors table

Old method did not accurately resolve jsdelivr urls for github repos with a supplied version. Hard coded `master` meta  pulling made the versioning obsolete, and repos without a `master` branch didn't work at all.

This update makes it possible to accurately pull specific release versions from GitHub repos, and the default branch is fetched from the repository meta.
@codesandbox
Copy link

codesandbox bot commented Nov 10, 2022

CodeSandbox logoCodeSandbox logo  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 738c96b:

Sandbox Source
Notifications Test Configuration
r3-rapier-branding-test PR

@lbogdan
Copy link
Contributor

lbogdan commented Nov 10, 2022

Build for latest commit 738c96b is at https://pr7133.build.csb.dev/s/new.

@wiledal wiledal changed the title Fix: accurately resolve GitHub versions in jsdelivr fetcher Fix(sandpack): accurately resolve GitHub versions in jsdelivr fetcher Nov 11, 2022
@wiledal wiledal changed the title Fix(sandpack): accurately resolve GitHub versions in jsdelivr fetcher fix(sandpack): accurately resolve GitHub versions in jsdelivr fetcher Nov 11, 2022
Copy link
Member

@CompuIves CompuIves left a comment

Choose a reason for hiding this comment

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

This looks really great! Thanks for diving into this and finding a solution!!

@CompuIves CompuIves merged commit 618a785 into codesandbox:master Nov 15, 2022
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.

4 participants