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

feat: WebExperiment class #152

Draft
wants to merge 36 commits into
base: main
Choose a base branch
from
Draft

feat: WebExperiment class #152

wants to merge 36 commits into from

Conversation

tyiuhc
Copy link
Collaborator

@tyiuhc tyiuhc commented Jan 2, 2025

Summary

Checklist

  • Does your PR title have the correct title format?
  • Does your PR have a breaking change?:

@tyiuhc tyiuhc marked this pull request as draft January 2, 2025 23:46
Copy link
Collaborator

@bgiori bgiori left a comment

Choose a reason for hiding this comment

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

Initial review, will review more soon.

packages/experiment-tag/src/experiment.ts Outdated Show resolved Hide resolved
return;

/**
* Set the previous URL for tracking back/forward navigation. Set previous URL to prevent infinite redirection loop
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a way we can prevent this for them?

packages/experiment-tag/src/experiment.ts Outdated Show resolved Hide resolved
packages/experiment-tag/src/experiment.ts Outdated Show resolved Hide resolved
packages/experiment-tag/src/experiment.ts Outdated Show resolved Hide resolved
* Get redirect URLs for flags.
* @param flagKeys
*/
public getRedirectUrls(
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the use case for this function?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is for the case where the developer want to implement their own redirect handler.

* @param key
* @param variant
*/
public previewVariant(key: string, variant: string) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Support previewing multiple flags/variants. Maybe we just merge this with applyVariants with another config like preview?

* Set URL change listener to revert mutations and apply variants on back/forward navigation.
* @param flagKeys
*/
public setUrlChangeListener(flagKeys: string[]) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this supposed to have a listener argument?

Base automatically changed from web-remote-eval to main January 6, 2025 23:24
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.

2 participants