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

In Bare, Linking crashes with null is not an object (evaluating 'manifest.hostUri') #10095

Closed
lc3t35 opened this issue Sep 8, 2020 · 11 comments · Fixed by #10195
Closed

In Bare, Linking crashes with null is not an object (evaluating 'manifest.hostUri') #10095

lc3t35 opened this issue Sep 8, 2020 · 11 comments · Fixed by #10195

Comments

@lc3t35
Copy link

lc3t35 commented Sep 8, 2020

🐛 Bug Report

Summary of Issue

In Bare (sdk38), Linking crashes with null is not an object (evaluating 'manifest.hostUri')

IMG_65A1588D585A-1

Environment - output of expo diagnostics & the platform(s) you're targeting

Expo CLI 3.26.2 environment info:
    System:
      OS: macOS 10.15.6
      Shell: 5.7.1 - /bin/zsh
    Binaries:
      Node: 12.13.1 - /usr/local/bin/node
      Yarn: 1.22.4 - /usr/local/bin/yarn
      npm: 3.10.10 - /usr/local/bin/npm
      Watchman: 4.9.0 - /usr/local/bin/watchman
    IDEs:
      Android Studio: 3.6 AI-192.7142.36.36.6392135
      Xcode: 11.6/11E708 - /usr/bin/xcodebuild
    npmPackages:
      expo: ^38.0.9 => 38.0.9
      react: ~16.11.0 => 16.11.0
      react-dom: 16.11.0 => 16.11.0
      react-native: ~0.62.2 => 0.62.2
      react-native-web: ~0.11.7 => 0.11.7
    npmGlobalPackages:
      expo-cli: 3.26.2

Reproducible Demo

Not easy as it's a bare project.
Similar issue #7807

Steps to Reproduce

Open the link, the app is launched, linking is handled by getInitialURL or addEventListener then parsing crashes

import * as Linking from 'expo-linking';
/../
let { path, queryParams } = Linking.parse(url);

url is correct : scheme://join?memberId=xxx

Expected Behavior vs Actual Behavior

Previous version with sdk35 did not crash.
As mentioned in expo-notification README,
"Defaults to [Constants.manifest.id](https://docs.expo.io/versions/latest/sdk/constants/#constantsmanifest) exposed by expo-constants. You may need to define it in bare workflow, where expo-constants doesn't expose the manifest."

https://github.com/expo/expo/blob/master/packages/expo-linking/src/Linking.ts imports expo-constants

import Constants from 'expo-constants';
/.../
const { manifest } = Constants;
/.../
function getHostUri(): string {
  if (!manifest.hostUri && !usesCustomScheme()) {                                  <<<<<<<<<<<<<< CRASH HERE
    // we're probably not using up-to-date xdl, so just fake it for now
    // we have to remove the /--/ on the end since this will be inserted again later
    return removeScheme(Constants.linkingUri).replace(/\/--($|\/.*$)/, '');
  }
  return manifest.hostUri;
}

So that's why it is broken in bare workflow ....

How can we urgently solve this ?

Quickfix until solved

I just need path and queryParams, so I replaced Linking.parse(url) with

export function fixLinkingParse(url) {
  // url is this kind for me : scheme://path?queryParams
  const parsed = URL(url, /* parseQueryString */ true);
  for (const param in parsed.query) {
    parsed.query[param] = decodeURIComponent(parsed.query[param]);
  }
  const queryParams = parsed.query;
  const path = parsed.host || null;
  return {
    path,
    queryParams
  };
}
@lc3t35 lc3t35 added the needs validation Issue needs to be validated label Sep 8, 2020
@lc3t35
Copy link
Author

lc3t35 commented Sep 11, 2020

This issue is open for a long time now (#7721 closed without a fix, #8973 assigned since August 11th to https://github.com/esamelson) ... what is the problem ?

@cruzach
Copy link
Contributor

cruzach commented Sep 14, 2020

Taking a look at this @lc3t35 , thanks for reporting

@cruzach cruzach added Bare Workflow Linking and removed needs validation Issue needs to be validated labels Sep 14, 2020
@barthap
Copy link
Contributor

barthap commented Sep 15, 2020

In Bare that if with manifest.hostUri is crashing:

function getHostUri(): string {
if (!manifest.hostUri && !usesCustomScheme()) {
// we're probably not using up-to-date xdl, so just fake it for now
// we have to remove the /--/ on the end since this will be inserted again later
return removeScheme(Constants.linkingUri).replace(/\/--($|\/.*$)/, '');
}
return manifest.hostUri;
}

Quick workaround would be checking if manifest exists. However this should also be done in usesCustomScheme() and probably a few other places.

-  if (!manifest.hostUri && !usesCustomScheme()) {
+  if (!(manifest?.hostUri) && !usesCustomScheme()) {

However, the method still accesses Constants.linkingUri which is also unavailable in Bare as we can see here.

@cruzach
Copy link
Contributor

cruzach commented Sep 15, 2020

Linking should still be usable in bare, and we can rely on Updates.manifest instead of Constants.manifest in the bare workflow, the issue is that builds will have an empty Updates.manifest until their first OTA update 🙁

@tylerc
Copy link

tylerc commented Sep 15, 2020

I'll chime in here and say that I'm facing this issue too. I'd been seeing some errors about it in Sentry for a few weeks and finally was able to track it down to this.

Honestly, for now I'm thinking I'll just stop using Linking.parse and rewrite my own version, not ideal but it doesn't look like there's any other way forward.

Btw, for other issues with bare that I've since forgotten I needed to monkey-patch Constants.manifest:

import * as manifest from '../../app.json';

if (!Constants.manifest) {
  Constants.manifest = {
    ...manifest.expo,
    id: '...',
  } as any;
}

But since Linking.ts uses destructuring, this approach doesn't fix it:

const { manifest } = Constants;

@rafatrace
Copy link

I have a similar error in SDK 39 Bare workflow:

 TypeError: null is not an object (evaluating 'Constants.manifest.logUrl')

To giver more information, I just ejected from expo and have zero experience with "pure" React Native development, only with Expo.

image

@brentvatne
Copy link
Member

update to latest expo-linking: 1.0.5

@rafatrace
Copy link

update to latest expo-linking: 1.0.5

Unfortunately that didn't fix the issue, it's still there. Notice that this is related to Constants.manifest.logUrl and not Constants.manifest.hostUri like the original error message of this thread.

@BazanAlexandro
Copy link

Yep, Constants.linkingUri is undefined for me in bare workflow, causing an error in getHostUri method of expo-linking
Screenshot 2020-11-16 at 16 58 42

@BazanAlexandro
Copy link

Error happens when calling Linking.parse, so, I think you should be fine if using url package for parsing instead

@brentvatne
Copy link
Member

sorry this issue has been closed for a while, if you're still encountering a similar problem then please create a new issue and fill out the issue template so we have enough information to help :)

@expo expo locked and limited conversation to collaborators Nov 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants