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

[App bridge] Add middleware to pass clientInterface data #1033

Merged
merged 1 commit into from
Feb 20, 2019

Conversation

iainmcampbell
Copy link
Contributor

WHY are these changes introduced?

https://github.com/Shopify/app-bridge/issues/737 (more context: https://github.com/Shopify/app-bridge/issues/730)

We'd like to be able to track additional information about how [App bridge] is being consumed. With the legacy EASDK we provided a way to include metadata about the interface which Polaris passed in. No such mechanism exists in App Bridge.

WHAT is this pull request doing?

This PR adds a tiny middleware which adds a clientInterface property to all App bridge actions dispatched from Polaris, so we can track whether people are using App Bridge through Polaris.

How to 🎩

Buckle up, this involves 3 repos 😅

Make sure Core & Web are running.

If you want to see the final Monorail events, you can check out app-bridge/version-tracking in Web, but it’s not strictly necessary.

In your local app-bridge repo, check out version-tracking-polaris-app (this changes one of the App bridge playground app’s pages to use App bridge through Polaris).

In Polaris-react, run yarn build-consumer app-bridge.

Back in your friendly neighbourhood app bridge repo, run dev up && dev server. If you haven't installed the App bridge playground app on your local shop1, go to https://app-bridge.myshopify.io/auth/shopify?shop=shop1.myshopify.io.

Visit https://shop1.myshopify.io/admin/apps/app-bridge/foo.

In the Redux dev tools, make sure you have the App Bridge instance selected (top right). Find the APP::TITLEBAR::UPDATE action. It should contain clientInterface info:

{
  ...
  clientInterface: {
    name: '@shopify/polaris',
    version: '3.7.0-rc.2'
  },
  ...
}

(make sure you have Action and Raw selected)

screen shot 2019-02-13 at 3 45 18 pm

If you checked out the web branch earlier, you should also be able to see a Monorail event with clientInterfaceName and clientInterfaceVersion properties.

screen shot 2019-02-13 at 3 52 46 pm

🎩 checklist

@ghost
Copy link

ghost commented Feb 13, 2019

👋 Thanks for opening your first pull request. A contributor should give feedback soon. If you haven’t already, please check out the contributing guidelines. You can also join #polaris on the Shopify Partners Slack.

@BPScott BPScott temporarily deployed to polaris-react-pr-1033 February 13, 2019 20:56 Inactive
Copy link
Contributor

@hannachen hannachen left a comment

Choose a reason for hiding this comment

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

Code look good to me 👍 🎩together with https://github.com/Shopify/web/pull/10560

Copy link
Member

@henrytao-me henrytao-me left a comment

Choose a reason for hiding this comment

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

🎩-ed. Looks good.

@iainmcampbell iainmcampbell merged commit 0df2fa6 into master Feb 20, 2019
@ghost
Copy link

ghost commented Feb 20, 2019

🎉 Thanks for your contribution to Polaris React!

@iainmcampbell iainmcampbell deleted the app-bridge-clientInterface branch February 20, 2019 22:18
@tmlayton
Copy link
Contributor

Would like to see better coverage and a changelog for this

@iainmcampbell
Copy link
Contributor Author

@tmlayton sure, I can do a follow up PR with better test coverage.

This is a super under-the-hood change that doesn’t affect the public API; it’s just to help us track how people are using App Bridge. I can still add a changelog entry though ("Send Polaris version information along with App Bridge actions").

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.

5 participants