Skip to content
This repository has been archived by the owner on Jan 18, 2024. It is now read-only.

Added currentFullName to the public config #3376

Merged
merged 2 commits into from
Apr 8, 2021

Conversation

EvanBacon
Copy link
Contributor

@EvanBacon EvanBacon commented Apr 8, 2021

Why

Test Plan

  • Added unit tests.
  • I've verified that the value currentFullName will be overwritten by the server so this shouldn't introduce a breaking change.
  • Unless we also add this value to expo.id, the auth session and notifications modules will need to be updated to use currentFullName.

@@ -96,6 +96,7 @@ describe(getConfig, () => {
// - generated `.expo` object is created and the language hint is added
describe('language support', () => {
beforeEach(() => {
delete process.env.EXPO_DEBUG;
Copy link
Member

Choose a reason for hiding this comment

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

we should probably do this before every test in the expo-cli repo, maybe in global setup. i noticed that we set EXPO_DEBUG on ci but people may not have that set locally, which is i guess what you ran into here too

Copy link
Member

Choose a reason for hiding this comment

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

i guess in general process.env should just have no contents unless we explicitly set them for specific tests

@EvanBacon EvanBacon merged commit d9d4248 into master Apr 8, 2021
@EvanBacon EvanBacon deleted the @evanbacon/config/add-currentFullName branch April 8, 2021 23:12
wschurman added a commit to expo/expo that referenced this pull request Feb 6, 2023
…rted config (#21070)

# Why

This reverts expo/expo-cli#3376 and
expo/expo-cli#3494.

These were added for AuthSession and Push notification registration.
Neither of these use this anymore:
- AuthSession deprecated use of the proxy use with the field from the
manifest: #17327. It supports supplying
them via arguments.
- Push notification prefers `projectId` now:
#14265. Also it allows supplying them
via arguments.

These auto config augmentations are causing issues with `eas build`
since the generated full name could differ from the actual full name:
1. Create project in an org on the website.
2. `npx expo init ...`
3. `eas init --id <>`
4. Remove `owner` from manifest.
5. `eas build`
6. Download artifact, inspect embedded config, see that it has the
correct `projectId` yet also a `currentFullName` and `originalFullName`
containing the current logged-in user rather than the actual one
corresponding to the projectId.

# How

One fix we could do here would be to read the projectId and get the
truthful full name, but that adds a network dependency and is probably
not worth it.

So, instead, I decided to just remove the hack since they are no longer
needed in the two libraries this was added for.

# Test Plan

Run all tests.

# Checklist

<!--
Please check the appropriate items below if they apply to your diff.
This is required for changes to Expo modules.
-->

- [ ] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [ ] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [ ] This diff will work correctly for `expo prebuild` & EAS Build (eg:
updated a module plugin).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants