Skip to content
This repository has been archived by the owner on May 20, 2024. It is now read-only.

isEligible should fire on route change #7

Closed
wants to merge 3 commits into from

Conversation

japboy
Copy link

@japboy japboy commented Mar 5, 2019

I need this Optimize module and want to run different experiments on each page, so I made #3 possible.

@@ -5,14 +5,23 @@ import experiments from '<%= options.experimentsDir %>'
const MAX_AGE = <%= options.maxAge %>

export default function (ctx, inject) {
ctx.app.router.beforeEach((to, from, next) => {
Copy link

Choose a reason for hiding this comment

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

Won't this override any existing function on beforeEach? Just wondering.

Copy link
Author

Choose a reason for hiding this comment

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

beforeEach is a register that adds callback function, so it won't override existing functions, i think.

https://github.com/vuejs/vue-router/blob/v3.0.2/src/index.js#L121-L123

Copy link

Choose a reason for hiding this comment

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

Looks like you're right, cool. Didn't know that :)

@japboy
Copy link
Author

japboy commented Mar 8, 2019

I think I need more considerations before this is accepted, so I close this PR. Sorry for bothering.

@japboy japboy closed this Mar 8, 2019
@japboy japboy deleted the track-on-route-change branch March 8, 2019 07:46
@japboy japboy restored the track-on-route-change branch March 8, 2019 07:46
@tschut
Copy link

tschut commented Mar 12, 2019

@japboy what's the problem? I'd really like to have this functionality too so would like to help if possible.

@japboy
Copy link
Author

japboy commented Mar 12, 2019

@tschut first of all, running multiple experiments at once is not ideal way of A/B testing or MVT. second, changing cookie format is necessary to support multiple experiment data to be stored properly. current implementation supports only one experiment at once. and i need some minor fixes if this is merged (like firing multiple route events at once etc...).

i think i should define MVT for multiple endpoint experiments instead of firing route event.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants