🚀 Move timezoneCode macro from url-replacement-impl #25597
🚀 Move timezoneCode macro from url-replacement-impl #25597micajuine-ho wants to merge 13 commits intoampproject:masterfrom
Conversation
zhouyx
left a comment
There was a problem hiding this comment.
You should also be able to remove the variable from the a4a-variable-source.js allowed list.
| } | ||
| return val; | ||
| // amp-analytics variable | ||
| switch (name) { |
There was a problem hiding this comment.
Why we need to handle the variable specially?
There was a problem hiding this comment.
In the test, it expands the variables out so ${timezoneCode} -> America%2FLos_Angeles instead of _timezone_code_.
So need to whitelist them here.
Other option would be to change everyone's vendor configs, but that wouldn't work, unless we faked all of these variables' expansions (i.e. faking ${timezoneCode} -> America%2FLos_Angeles, no matter where you're testing from).
There was a problem hiding this comment.
Feels like we should start to mock those variables from getMacros() just like variables from the global list now.
There was a problem hiding this comment.
Sounds like the right approach. Will do that first.
|
@zhouyx should we also remove them them from |
ad7a0a7 to
64360ba
Compare
|
i looked at the implementation, the |
43d137b to
cf95db4
Compare
b521a84 to
6b0347d
Compare
Project Tracker: #26091
Moving timezoneCode macro to amp-analytics.