react-native\packages\react-native\Libraries\Blob\URL.js Changes #48333
Conversation
|
Hi @bobbysharma05! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
facebook-github-bot
added
CLA Signed
|
Hi @bobbysharma05, thanks for working on this. |
cipolleschi
left a comment
cipolleschi
left a comment
There was a problem hiding this comment.
Code changes look good to me, but the PR is missing information in the Description.
We can't import without Summary, Changelog and TestPlan
|
Also, the test_js signal is failing, can you have a look at it? |
|
ok sir I will look into it and also add the summary. |
|
Sir I am new to the open source. Could you please help me in which part I am getting stuck. |
|
When a new pr is open, we provide a template that has to be filled. For the Changelog, specifically, it has to follow a predefined format, which is described here. For this pr, it should be something like: For the failures, you can click on the failed signals in the github UI and read through the logs, looking for the errors (the search function is your friend). For example, the linter says that the community-cli-plugin has been built and it should not. If you inspect the changed files, you see that you committed a package.json that should not be committed. The test_js says that there is an invalid expression at line 25 of the URL.js file. Build_android failure is related to the failure for the linter, so it should be fixed by reverting the changes in the |
|
also related PR: #48505 |
|
@cipolleschi can you review the PR which I've submitted? |
|
/rebase - this comment automatically rebase on top of main |
cipolleschi
left a comment
cipolleschi
left a comment
There was a problem hiding this comment.
@bobbysharma05, thanks for the patience waiting for the review.
The code currently does not build, there are some syntax error that needs to be fixed.
| function validateBaseUrl(url: string): boolean { | ||
| // from this MIT-licensed gist: https://gist.github.com/dperini/729294 | ||
| return /^(?:(?:(?:https?|ftp):)?\/\/)(?:(?:[1-9]\d?|1\d\d|2[01]\d|22[0-3])(?:\.(?:1?\d{1,2}|2[0-4]\d|25[0-5])){2}(?:\.(?:[1-9]\d?|1\d\d|2[0-4]\d|25[0-4]))|(?:(?:[a-z0-9\u00a1-\uffff][a-z0-9\u00a1-\uffff_-]{0,62})?[a-z0-9\u00a1-\uffff]\.)*(?:[a-z\u00a1-\uffff]{2,}\.?))(?::\d{2,5})?(?:[/?#]\S*)?$/.test( | ||
| return /^(?:(?:(?:https?|ftp):)?\/\/)(?:(?:[1-9]\d?|1\d\d|2[01]\d|22[0-3])(?:\.(?:1?\d{1,2}|2[0-4]\d|25[0-5])){2}(?:\.(?:[1-9]\d?|1\d\d|2[0-4]\d|25[0-4]))|(?:(?:[a-z0-9\u00a1-\uffff][a-z0-9\u00a1-\uffff_-]{0,62})?[a-z0-9\u00a1-\uffff]\.)(?:[a-z\u00a1-\uffff]{2,}\.?))(?::\d{2,5})?(?:[/?#]\S)?$/.test( |
There was a problem hiding this comment.
why did you removed the *?
| throw new Error('Invalid blob data: Missing blobId or data.'); | ||
| } | ||
| return `${BLOB_URL_PREFIX}${blob.data.blobId}?offset=${blob.data.offset}&size=${blob.size}`; | ||
| return ${BLOB_URL_PREFIX}${blob.data.blobId}?offset=${blob.data.offset}&size=${blob.size}; |
There was a problem hiding this comment.
missing `. This is not valid JS code
| baseUrl = base; | ||
| if (!validateBaseUrl(baseUrl)) { | ||
| throw new TypeError(`Invalid base URL: ${baseUrl}`); | ||
| throw new TypeError(Invalid base URL: ${baseUrl}); |
There was a problem hiding this comment.
Backticks ` are needed here as well
| } | ||
| if (!url.startsWith('/')) { | ||
| url = `/${url}`; | ||
| url = /${url}; |
| url = ''; | ||
| } | ||
| this._url = `${baseUrl}${url}`; | ||
| this._url = ${baseUrl}${url}; |
|
Closing in favor of #49955 |
5 participants
SUMMARY
The URL class was refactored to handle blob URLs more effectively, improving URL construction and parsing. Changes include better error handling in the createObjectURL method and the addition of functions to extract URL components like username, host, and protocol.
Fixes #45030
Changelog:
[General][Added] - Implement missing methods for URL
Test Plan: