URL.href has an extraneous slash

[fabOnReact]

Summary

This issue fixes #30914
The logic of adding extraneous slash to url was introduced with pr #22901 but It should be removed for the following reasons:

• does not respect the WHATWG URL standard
• the logic was introduced with Flesh out the URL polyfill a bit more #22901 to add compatability with apollo-server Polyfill for URL is incomplete and causes crash with apollo-server #22594 but apollo-server seems to already add the slash to the end of urls

Changelog

[Android] [Fixed] - Avoid adding extraneous slash to end of url

Test Plan

TODO:

• increase test coverage for URL: Do not prepend baseUrl if the URL is not a relative URL #26009 URL with base is always used, even when absolute URL is provided #26006 URL cannot handle "localhost" domain for base url #26019 Implement URL standard and URLSearchParams spec compliant #25719

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: ede065c

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,903,236	-246
android	hermes	armeabi-v7a	8,400,395	-253
android	hermes	x86	9,392,274	-245
android	hermes	x86_64	9,336,368	-246
android	jsc	arm64-v8a	10,637,277	-280
android	jsc	armeabi-v7a	10,117,539	-276
android	jsc	x86	10,687,830	-267
android	jsc	x86_64	11,272,185	-273

Base commit: ede065c

[fabOnReact]

TODO

• understand reason we would add extraneous slash with URL: Do not prepend baseUrl if the URL is not a relative URL #26009
  The logic was introduced with Flesh out the URL polyfill a bit more #22901 to add compatability with apollo-server Polyfill for URL is incomplete and causes crash with apollo-server #22594 but apollo-server seems to already add the slash to the end of urls
• test react-native with apollo (optional)
• increase test coverage for URL: Do not prepend baseUrl if the URL is not a relative URL #26009 URL with base is always used, even when absolute URL is provided #26006 URL cannot handle "localhost" domain for base url #26019 Implement URL standard and URLSearchParams spec compliant #25719

[yungsters]

I left a comment here: #30914 (comment)

While this does fix a bug, it introduces another bug. If we are going to make a change to URL at this point (which would be a breaking change for someone), we should probably go for the correct behavior.

I believe the correct behavior will require parsing supplied URLs into each parts in order to do proper path formatting (and / insertion).

[fabOnReact]

Thanks a lot Timothy Yung.

I include this information to help myself troubleshoot this issue.
We currently don't compute the base when running this test

    const b = new URL('https://developer.mozilla.org');
    expect(b.href).toBe('https://developer.mozilla.org/');

Instead we just add the / at the end of the entire string/url, without identifying baseUrl and path.

react-native/Libraries/Blob/URL.js

Lines 127 to 133 in f602cd9

	constructor(url: string, base: string) {
	let baseUrl = null;
	if (!base || validateBaseUrl(url)) {
	this._url = url;
	if (!this._url.endsWith('/')) {
	this._url += '/';
	}

I am looking into the whatwg-url URL-impl, debugging basicURLParse by running the test suite, reading the specs and hopefully I will be able to add it soon to react-native.

Thanks a lot 🙏

[fabOnReact]

whatwg-url uses parse host, parseHost functions to parse the host from the url.

Other functions will handle other required aspects of parsing and formatting urls.

#22901 (comment)

This looks fine for me for now. I wonder if there is a third-party dep for URL we could use instead of maintaining our own copy?

I'm considering using basicURLParse imported via the whatwg-url npm package to parse supplied URLs into each parts in order to do proper path formatting (and / insertion).

Thanks a lot 🙏

[yungsters]

If we're going to pull in whatwg-url, would it make sense to directly depend on its URL implementation entirely?

My only concern would be the size of the library introduced into React Native. On the one hand… this module should only need to be initialized if it is used. On the other hand… it still introduces weight to app binaries.

Taking a step back, though… since we already provide URL, we owe it to the platform to provide a proper implementation of it. (If app size were a concern, I propose that we tackle that as a separate follow-up discussion.)