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

URL.searchParams.has method is broken #33336

Open
federico-hv opened this issue Dec 1, 2024 · 3 comments
Open

URL.searchParams.has method is broken #33336

federico-hv opened this issue Dec 1, 2024 · 3 comments
Assignees
Labels
needs review Issue is ready to be reviewed by a maintainer

Comments

@federico-hv
Copy link

Minimal reproducible example

https://gist.github.com/federico-hv/8a5459b23867635b66f97e0605e73b57

What platform(s) does this occur on?

iOS

Where did you reproduce the issue?

in a development build

Summary

Using the URL web api to validate a search param I was getting a wrong value and realized that the method url.searchParams.has returns true even when an incorrect value is given. I saw that expo apparently uses this implementation of URL https://github.com/charpeni/whatwg-url here https://github.com/expo/expo/blob/sdk-52/packages/expo/src/winter/url.ts so I tried whatwg-url-without-unicode and it seems the bug is on their side. I reported the bug here charpeni/whatwg-url#7 and create a gist here to explain it https://gist.github.com/federico-hv/8a5459b23867635b66f97e0605e73b57

Environment

expo-env-info 1.2.1 environment info:
    System:
      OS: macOS 14.0
      Shell: 5.9 - /bin/zsh
    Binaries:
      Node: 23.3.0 - ~/.nvm/versions/node/v23.3.0/bin/node
      Yarn: 4.5.2-git.20241122.hash-2f490dd - ~/.nvm/versions/node/v23.3.0/bin/yarn
      npm: 10.9.0 - ~/.nvm/versions/node/v23.3.0/bin/npm
    Managers:
      CocoaPods: 1.15.2 - /Users/federicohernandezvelasquez/.rvm/gems/ruby-2.7.6/bin/pod
    SDKs:
      iOS SDK:
        Platforms: DriverKit 23.2, iOS 17.2, macOS 14.2, tvOS 17.2, watchOS 10.2
    IDEs:
      Android Studio: 2022.2 AI-222.4459.24.2221.9862592
      Xcode: 15.1/15C65 - /usr/bin/xcodebuild
    npmPackages:
      expo: ~52.0.11 => 52.0.11 
      expo-router: ~4.0.9 => 4.0.9 
      react: 18.3.1 => 18.3.1 
      react-dom: 18.3.1 => 18.3.1 
      react-native: 0.76.3 => 0.76.3 
      react-native-web: ~0.19.13 => 0.19.13 
    Expo Workflow: bare

Expo Doctor Diagnostics

Enabled experimental React Native Directory checks. Unset the EXPO_DOCTOR_ENABLE_DIRECTORY_CHECK environment variable to disable this check.
✔ Check package.json for common issues
✔ Check Expo config for common issues
✔ Check if the project meets version requirements for submission to app stores
✖ Check for common project setup issues
✔ Check dependencies for packages that should not be installed directly
✔ Check for issues with Metro config
✖ Check for app config fields that may not be synced in a non-CNG project
✔ Validate packages against React Native Directory package metadata
✔ Check Expo config (app.json/ app.config.js) schema
✔ Check npm/ yarn versions
✔ Check for legacy global CLI installed locally
✔ Check that native modules do not use incompatible support packages
✔ Check native tooling versions
✔ Check that native modules use compatible support package versions for installed Expo SDK
✔ Check that packages match versions required by installed Expo SDK

Detailed check results:

Multiple lock files detected (yarn.lock, package-lock.json). This may result in unexpected behavior in CI environments, such as EAS Build, which infer the package manager from the lock file.

This project contains native project folders but also has native configuration properties in app.json, indicating it is configured to use Prebuild. When the android/ios folders are present, if you don't run prebuild in your build pipeline, the following properties will not be synced: orientation, icon, scheme, userInterfaceStyle, ios, android, plugins, androidStatusBar.
@federico-hv federico-hv added the needs validation Issue needs to be validated label Dec 1, 2024
@expo-bot expo-bot added needs review Issue is ready to be reviewed by a maintainer and removed needs validation Issue needs to be validated labels Dec 1, 2024
@ashxjs
Copy link

ashxjs commented Dec 3, 2024

Hi 👋,

This may be related to this issue: facebook/react-native#45030

There is a opened PR related to this issue: facebook/react-native#45055

But still not merged :/

@marklawlor marklawlor assigned EvanBacon and unassigned marklawlor Dec 3, 2024
@marklawlor
Copy link
Contributor

As explained by the OP, this is an upstream issue with a dependency that we use. I believe that we only use this within the wintercg environments (API routes, RSC, SSR, etc).

Please follow the issue linked by the OP for updates. We will update the dependency when they release a new version.

@federico-hv
Copy link
Author

federico-hv commented Dec 6, 2024

@ashxjs thanks for the reference. It seems that the support for web compatible APIs in the react-native ecosystem is indeed an issue and that URL and URLSearchParams objects are not supported neither on Hermes (also missing in jscore apparently) nor the react-native library itself. This is as mentioned by the Hermes team because URL is part of the Web APIs and not ECMAScript and Hermes is not supposed to run in Browsers. I assume that's the same rationale on the react-native team or it just doesn't have priority. This is why the community tends to go with solutions like react-native-url-polyfill which uses this fork of jsdom's whatwg-url but without the unicode and works in many cases like when adding a Supabase client but I just found it could potentially break others like this one that I discovered while trying to use ElectricSQL.

I was trying Expo with create-expo-app and the latest SDK and it seems that the value that I'm getting is still wrong which is why I'm wondering if I may be missing something in order to make this work properly or this is definitely an issue and library developers shouldn't expect the URL object to be implemented completely. I just expected it to be functional after reading the Expo URL doc so it's a bit confusing.

Screenshot 2024-12-05 at 7 06 01 PM

Hermes Engine Issues

React Native Library Issues

Other react-native-url-polyfill errors

charpeni/react-native-url-polyfill#471

Maybe there should be a bigger warning when referencing this polyfill?

Expo Docs (check STEP 2) includes the polyfill

https://docs.expo.dev/guides/using-supabase/

Supabase docs (check STEP4) includes polyfill

https://supabase.com/docs/guides/auth/quickstarts/react-native

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review Issue is ready to be reviewed by a maintainer
Projects
None yet
Development

No branches or pull requests

5 participants