✨ Add requestOrigin field in analytics config #22454
Conversation
|
Discussed offline. IE11 does not support URL constructor and its behaviour is not exactly what we want. Will need to use custom URL constructor. |
|
cc @zhouyx ready for review! |
|
Discussed offline.
E.g. "https://example.com/test1/test2" -> "https://example.com" If An |
|
cc @zhouyx ready for review! |
| yield macroTask(); | ||
| expect(spy).to.be.calledWith('http://example.com/r3'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
We talked about this, but I still think it's worth adding the requestOrigin being relative url test. Tests are mostly for developers, and it helps them to understand the desired behavior much better. With that test, we could save a lot of explanation on how we want to handle the special case where requestOrigin is not absolute. WDYT?
|
Ping @jonathantyng-amp let me know once you think the PR is ready : ) Thank you |
|
cc @zhouyx ready for review! |
|
@jonathantyng-amp got green build! Merged! 🎉 🎉 🎉 |
* add request origin * expansionOptions fix * Add changes from comments * fix tests * typo * use URL constructor and move requestOrigin propagation to config.js * catch URL construct error * add custom url constructor * cleanup unused code * changes from comments * Don't extract relative path from request and support origin field in request object * remove unused tests * add new tests and use URL constructor * use UrlService * add comments and tests * lint
Add a new requestOrigin field to the top level analytics config. The requestOrigin part will not be encoded and all outgoing URLs will be prepended with it.
See #19759 for more details