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

[constants][asset] Suppress warning upon no manifest available #12237

Merged

Conversation

bbarthec
Copy link
Contributor

@bbarthec bbarthec commented Mar 17, 2021

Why

Resolves https://linear.app/expo/issue/ENG-576/make-expo-asset-not-warn-about-missing-manifest
Resolves #11888
Supersedes #12230

When in bare workflow a user might encounter scenario when there's no
manifest available. expo-asset tries to read manifest anyway and it triggers the warning,
even though expo-asset can manage without Constants.manifest.
This change fixes that scenario and turns off this no-op warning.

How

  • Deliver new Constants.__unsefaNoWarnManifest property that behaves exactly the same
    as Constants.manifest, but prevent the warning about nullish manifest from being printed.
  • Use this new property in expo-asset and in logging.fx in expo package.

Test Plan

Copied the changes to the bare workflow project having this problem and ensured the warning is no longer there 😉

@bbarthec bbarthec force-pushed the @bbarthec/constants/asset/suppress-warning-upon-no-manifest branch from f396764 to 3e3b1e9 Compare March 17, 2021 13:39
@bbarthec bbarthec marked this pull request as ready for review March 17, 2021 13:40
@bbarthec bbarthec requested review from ide and tsapeta as code owners March 17, 2021 13:40
@bbarthec bbarthec requested review from esamelson and brentvatne and removed request for ide and tsapeta March 17, 2021 13:40
@bbarthec bbarthec force-pushed the @bbarthec/constants/asset/suppress-warning-upon-no-manifest branch from 3e3b1e9 to 5afffd5 Compare March 17, 2021 13:44
Deliver new `Constants.__unsefaNoWarnManifest` property that behaves exaclty the same
as `Constants.manifest`, but prevent the warning about nullish manifest from being printed.

When in bare workflow a user might encounter scenario when there's no
manifest available. `expo-asset` tries to read manifest anyway and it triggers the warning,
even though `expo-asset` can manage without `Constants.manifest`.
This change fixes that scenario and turns off this no-op warning.
@bbarthec bbarthec force-pushed the @bbarthec/constants/asset/suppress-warning-upon-no-manifest branch from 5afffd5 to e940205 Compare March 17, 2021 14:17
Copy link
Contributor

@esamelson esamelson left a comment

Choose a reason for hiding this comment

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

this lgtm, thanks @bbarthec 🙏

Copy link
Member

@brentvatne brentvatne left a comment

Choose a reason for hiding this comment

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

👍

@brentvatne brentvatne merged commit 04726c8 into master Mar 17, 2021
@brentvatne brentvatne deleted the @bbarthec/constants/asset/suppress-warning-upon-no-manifest branch March 17, 2021 22:15
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.

expo-asset relies on Constants.manifest and throws yellowbox warnings in bare projects
3 participants