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

[notifications] add 'assert' as a dependency #11171

Merged
merged 2 commits into from
Nov 30, 2020

Conversation

cruzach
Copy link
Contributor

@cruzach cruzach commented Nov 30, 2020

Why

Without this dependency, SDK 40 projects will fail after starting the bundler with:

Unable to resolve module `assert` from `node_modules/@ide/backoff/build/backoff.js`: assert could not be found within the project.

How

yarn add assert

Test Plan

yarn add local/path/to/expo-notifications && expo publish succeeds

@cruzach cruzach requested a review from sjchmiela as a code owner November 30, 2020 21:34
@cruzach cruzach changed the title [notification] add 'assert' as a dependency [notifications] add 'assert' as a dependency Nov 30, 2020
@github-actions
Copy link
Contributor

Native Component List for this branch is ready

Copy link
Contributor

@sjchmiela sjchmiela left a comment

Choose a reason for hiding this comment

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

I wonder why I haven't gotten it while trying expo-notifications out in NCL… Could it be because jest-expo-puppeteer transiently depends on assert?

Since you've tested this change and it worked as expected in NCL which used assert too — thank you for taking care of this and sorry for not catching that earlier! 🥇

@brentvatne
Copy link
Member

@sjchmiela - yeah it's hard to catch missing dependency issues within the expo/expo monorepo because as long as any other package in the repo includes the dep then it'll be resolved 😱

@brentvatne brentvatne merged commit 2815646 into master Nov 30, 2020
@brentvatne brentvatne deleted the @cruzach/notifications-assert branch November 30, 2020 22:03
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.

3 participants