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

[Bug]: Customer.IO overwrites project file changes from other plugins #72

Closed
euc-callum opened this issue May 15, 2023 · 6 comments · Fixed by #76
Closed

[Bug]: Customer.IO overwrites project file changes from other plugins #72

euc-callum opened this issue May 15, 2023 · 6 comments · Fixed by #76

Comments

@euc-callum
Copy link
Contributor

euc-callum commented May 15, 2023

Hi,

We're attempting to utilise expo-notifications alongside customerio/customerio-expo-plugin for richpush notifications on iOS.

Whilst investigating writing our own Expo plugin to handle registering the entitlement file in the project file (a task on the Customer.IO roadmap with no timeline), we discovered this is impossible due to the manner in which Customer.IO is writing project files changes. We also discovered that it shouldn't be necessary as expo-notifications is already configuring it, but Customer.IO is then overwriting those changes.

I believe this would also impact any other plugin that modifies the iOS project files that exists in a project utilising customerio/customerio-expo-plugin, potentially overwriting any changes they make, which adds a lot of risk to utilising customerio/customerio-expo-plugin.

We believe the problems to be as follows:

Firstly, the plugin does not await asynchronous actions within it, and thus rather than running in sequence with other plugins that are modifying the project file, it will instead overlap them. This will result in race conditions where the behaviour of plugins will vary based on the timing of when the project file is read by a given plugin.

See here for an example that should be promisified. I've included what this would look like below:

await new Promise((resolve, reject) => {
  xcodeProject.parse(async function (err: Error) {
    if (err) {
      reject(`Error parsing iOS project: ${JSON.stringify(err)}`);
    }

    if (options.pushNotification) {
      await addPushNotificationFile(options, xcodeProject);
    }

    if (options.pushNotification?.useRichPush) {
      await addRichPushXcodeProj(options, xcodeProject);
    }

    FileManagement.writeFile(projPath, xcodeProject.writeSync());
    resolve();
  });
});

This change alone will not resolve things however, and will actually result in Customer.IO not working, as the changes that Customer.IO has then written to the project file will then be overwritten by Expo. See here for information on how Expo suggests to modify files via returning modResults rather than writing to files.

I believe Customer.IO's Expo plugin code needs modifying to utilise withXcodeProject and other similar variants rather than directly modifying files in order for it to work correctly, alongside ensuring it only returns once all asynchronous actions have completed.

@euc-callum
Copy link
Contributor Author

On top of this, we're currently attempting to use expo-notifications and Customer.IO (without rich push), and we're seeing the following email from Apple sporadically on iOS submissions:

ITMS-90078: Missing Push Notification Entitlement - Your app appears to register with the Apple Push Notification service, but the app signature's entitlements do not include the 'aps-environment' entitlement.

This looks to be due to the above issue in which this plugin is sporadically overwriting the config that expo-notifications writes, specifically in relation to the location of the entitlements file:

CODE_SIGN_ENTITLEMENTS

@euc-callum
Copy link
Contributor Author

An alternative to the promisify could be xcodeProject.parseSync().

@euc-callum
Copy link
Contributor Author

euc-callum commented May 15, 2023

I've raised a pull request with the changes I believe are needed to resolve this.

#73

@ami-aman
Copy link
Contributor

ami-aman commented May 15, 2023

I apologize for the inconvenience you are experiencing while using our plugin. Firstly, I would like to express my appreciation for providing such a detailed description of the issue. Your insights have greatly helped us in better understanding the issue at hand. We have encountered several compatibility issues recently, particularly with regards to our plugin and expo-notifications.

Please be assured that our team is actively working on addressing these issues and making the necessary adjustments to ensure compatibility between our plugin and expo-notifications. We are dedicated to providing a seamless experience for our users and appreciate your patience as we work towards a resolution.

Thanks for opening the PR. Your efforts to make our plugin better are well appreciated. Our team will look into it.

@euc-callum
Copy link
Contributor Author

For now we've forked the repository and pushed our own version to our internal NPM to unblock ourselves. Hopefully we'll switch back once this is resolved 👍

@sergey-king
Copy link

Also need this

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 a pull request may close this issue.

3 participants