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

amp-analytics: Shared interpolation & ampState() URL variable #13079

Closed
1 of 2 tasks
dreamofabear opened this issue Jan 27, 2018 · 9 comments
Closed
1 of 2 tasks

amp-analytics: Shared interpolation & ampState() URL variable #13079

dreamofabear opened this issue Jan 27, 2018 · 9 comments

Comments

@dreamofabear
Copy link

dreamofabear commented Jan 27, 2018

  • Extract amp-analytics variable interpolation parsing (${var}) so it can be shared with another extension
  • Implement ${ampState(foo.bar)} variable that reads from amp-bind state

/cc @lannka

@lannka
Copy link
Contributor

lannka commented Jan 28, 2018

The variable code is here

expandTemplate is the main function. Right now you can ignore any filter related code as they are dead code and @calebcordry is cleaning up.

To be clear, this VariableService code is to provide recursive var substitution to JSON config:

{
  vars: {
    varA: "123",
    varB: "${varA}"
  },

  requests: {
     a: "${varA}&${varB}"
  }
}

if you do expandTemplate(requests.a), it will return a promise of 123&123.

Another way to do similar thing is to use urlReplacementService, of which the code is already in core. To get what you want, you will add an AMP_STATE macro to url-replacements-impl.js, and use it like:

{
  requests: {
   a: "AMP_STATE(x)"
  }
}

I think urlReplacementService might be a better choice at this point, despite its name does not sound relevant.

@dreamofabear
Copy link
Author

Thanks for the pointers Hongfei. Let's work with the requesting team and see if they can contribute this if possible.

@calebcordry
Copy link
Member

calebcordry commented Jan 29, 2018

I agree with Hongfei. If you want to point them at the specific place where the Macros are defined it is here. That is a simple one but others can get fairly complex.

I have been recently been spending a lot of time with this code so let me know if I can help.

@jamesshannon
Copy link
Contributor

It looks like this is the FR to support amp-bind state variables in amp-analytics. If so, +1.

Use case: Ecommerce partner has a shopping cart and wants to set up an analytics ping for add-to-cart. That ping would include the product ID and price (which can be SSR'ed into the page) and also # of items being added.

One alternative way to implement this might be to allow data-vars-* to be bindable. E.g [data-vars-num-products]="cart[count]".

@dreamofabear
Copy link
Author

@ampprojectbot
Copy link
Member

This issue hasn't been updated in awhile. Do you have any updates?

@zhouyx
Copy link
Contributor

zhouyx commented Jul 2, 2019

Is there any work left here?

@dreamofabear
Copy link
Author

I believe sharing variable interpolation code between extensions is not done yet. I think priority can be dropped though.

@stale
Copy link

stale bot commented Dec 24, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale Inactive for one year or more label Dec 24, 2020
@stale stale bot closed this as completed Dec 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants