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
2 participants