Skip to content

Add vue router guard #12

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

michaelrodriguezGL
Copy link

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions. (Which are the supported platforms?)

Related issues
None.

Describe the solution you've provided

  • These changes enable guard usage within the Vue Router library.
  • It makes available the Launch Darkly instance which enables cross-sharing functionality.
  • It makes available the Launch Darkly instance readiness which enables cross-sharing functionality.
  • launchDarklyGuard:
    • Can be used to check a specific flag value and avoid page interaction if the result is false
    import { launchDarklyGuard } from "launchdarkly-vue-client-sdk";
    
    export const routes = [
      {
        path: "/my-route",
        name: "my-route",
        component: MyComponent,
        beforeEnter: launchDarklyGuard("CUSTOM_KEY"),
      },
    ];
    • Can be used to apply custom logic by passing a callback parameter:
    import { launchDarklyGuard } from "launchdarkly-vue-client-sdk";
    
    export const launchDarklyCustomGuard = launchDarklyGuard("CUSTOM_KEY", (flag, to, from, next) => {
      /** to, from, next <-- Values from router https://router.vuejs.org/api/interfaces/NavigationGuard.html */
    
      if (flag) { /** <-- returned value from Launch Darkly */
        return /** Here you can return true, false, or a route */
      }
    
      return /** Here you can return true, false, or a route */
    });

Describe alternatives you've considered
Component-based guard https://github.com/dashhudson/vue-ld#ldrouteguard-component

Additional context
Guard-based validation helps to reduce the amount of feature flags logic across components, it gives a centralized way of controlling page access.

I hope this sparks a conversation around Vue guards in LD :)

@michaelrodriguezGL michaelrodriguezGL marked this pull request as ready for review January 30, 2023 23:48
@yusinto yusinto self-requested a review January 31, 2023 00:04
@yusinto
Copy link
Contributor

yusinto commented Feb 1, 2023

Thank you for the PR @michaelrodriguezGL. We will review and discuss internally and respond again soon.

@michaelrodriguezGL
Copy link
Author

Thank you for the PR @michaelrodriguezGL. We will review and discuss internally and respond again soon.

Thanks, @yusinto.

@michaelrodriguezGL
Copy link
Author

Hi @yusinto, is there any news?

@michaelrodriguezGL
Copy link
Author

Hi @yusinto, is there any news? Anything I can help with?

@yusinto
Copy link
Contributor

yusinto commented Feb 27, 2023

Hi @michaelrodriguezGL apologies for the delay. We will take a look at this this week or the next. Thank you for your patience.

@michaelrodriguezGL
Copy link
Author

Hi @yusinto, thank you! I'm looking forward to your feedback

@louis-launchdarkly
Copy link
Contributor

Hello @michaelrodriguezGL, the team had just discussed this and this seems like a good idea. As you may have noticed from other LaunchDarkly SDKs, we are in the progress of doing major releases to support Context, so we are considering adopting this change when we start the Vue SDK major version work. For now, we have put this on the Vue SDK backlog.

Filed internally as 192072.

@michaelrodriguezGL
Copy link
Author

@louis-launchdarkly Sounds good, thanks for the update.

@sk-tjdownes
Copy link

There hasn't been any movement on this issue in a while, but I think this is important to Vue engineers using LD. Being able to protect an entire route seems a lot easier than flagging individual components.

@clarepure
Copy link

Would be beneficial to have this

@Hawxy
Copy link

Hawxy commented Oct 19, 2023

@louis-launchdarkly Context has been released for a while now, is there any update on this?

LaunchDarklyReleaseBot pushed a commit that referenced this pull request Dec 13, 2023
Bumps [semver](https://github.com/npm/node-semver) from 6.3.0 to 6.3.1.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/npm/node-semver/releases">semver's
releases</a>.</em></p>
<blockquote>
<h2>v6.3.1</h2>
<h2><a
href="https://github.com/npm/node-semver/compare/v6.3.0...v6.3.1">6.3.1</a>
(2023-07-10)</h2>
<h3>Bug Fixes</h3>
<ul>
<li><a
href="https://github.com/npm/node-semver/commit/928e56d21150da0413a3333a3148b20e741a920c"><code>928e56d</code></a>
<a href="https://redirect.github.com/npm/node-semver/pull/591">#591</a>
better handling of whitespace (<a
href="https://redirect.github.com/npm/node-semver/issues/591">#591</a>)
(<a href="https://github.com/lukekarrys"><code>@​lukekarrys</code></a>,
<a href="https://github.com/joaomoreno"><code>@​joaomoreno</code></a>,
<a
href="https://github.com/nicolo-ribaudo"><code>@​nicolo-ribaudo</code></a>)</li>
</ul>
</blockquote>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/npm/node-semver/blob/v6.3.1/CHANGELOG.md">semver's
changelog</a>.</em></p>
<blockquote>
<h2><a
href="https://github.com/npm/node-semver/compare/v6.3.0...v6.3.1">6.3.1</a>
(2023-07-10)</h2>
<h3>Bug Fixes</h3>
<ul>
<li><a
href="https://github.com/npm/node-semver/commit/928e56d21150da0413a3333a3148b20e741a920c"><code>928e56d</code></a>
<a href="https://redirect.github.com/npm/node-semver/pull/591">#591</a>
better handling of whitespace (<a
href="https://redirect.github.com/npm/node-semver/issues/591">#591</a>)
(<a href="https://github.com/lukekarrys"><code>@​lukekarrys</code></a>,
<a href="https://github.com/joaomoreno"><code>@​joaomoreno</code></a>,
<a
href="https://github.com/nicolo-ribaudo"><code>@​nicolo-ribaudo</code></a>)</li>
</ul>
<h2>6.2.0</h2>
<ul>
<li>Coerce numbers to strings when passed to semver.coerce()</li>
<li>Add <code>rtl</code> option to coerce from right to left</li>
</ul>
<h2>6.1.3</h2>
<ul>
<li>Handle X-ranges properly in includePrerelease mode</li>
</ul>
<h2>6.1.2</h2>
<ul>
<li>Do not throw when testing invalid version strings</li>
</ul>
<h2>6.1.1</h2>
<ul>
<li>Add options support for semver.coerce()</li>
<li>Handle undefined version passed to Range.test</li>
</ul>
<h2>6.1.0</h2>
<ul>
<li>Add semver.compareBuild function</li>
<li>Support <code>*</code> in semver.intersects</li>
</ul>
<h2>6.0</h2>
<ul>
<li>
<p>Fix <code>intersects</code> logic.</p>
<p>This is technically a bug fix, but since it is also a change to
behavior
that may require users updating their code, it is marked as a major
version increment.</p>
</li>
</ul>
<h2>5.7</h2>
<ul>
<li>Add <code>minVersion</code> method</li>
</ul>
<h2>5.6</h2>
<ul>
<li>Move boolean <code>loose</code> param to an options object, with
backwards-compatibility protection.</li>
<li>Add ability to opt out of special prerelease version handling with
the <code>includePrerelease</code> option flag.</li>
</ul>
<h2>5.5</h2>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/npm/node-semver/commit/44d27bc007e4827e9b797d6145f1076c127005f2"><code>44d27bc</code></a>
chore: release 6.3.1</li>
<li><a
href="https://github.com/npm/node-semver/commit/928e56d21150da0413a3333a3148b20e741a920c"><code>928e56d</code></a>
fix: better handling of whitespace (<a
href="https://redirect.github.com/npm/node-semver/issues/591">#591</a>)</li>
<li><a
href="https://github.com/npm/node-semver/commit/39f632690ea5b1b0d64fa913aa0f96f42b9bde32"><code>39f6326</code></a>
chore: <code>@​npmcli/template-oss</code><a
href="https://github.com/4"><code>@​4</code></a>.16.0</li>
<li>See full diff in <a
href="https://github.com/npm/node-semver/compare/v6.3.0...v6.3.1">compare
view</a></li>
</ul>
</details>
<details>
<summary>Maintainer changes</summary>
<p>This version was pushed to npm by <a
href="https://www.npmjs.com/~lukekarrys">lukekarrys</a>, a new releaser
for semver since your current version.</p>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=semver&package-manager=npm_and_yarn&previous-version=6.3.0&new-version=6.3.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

You can trigger a rebase of this PR by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
You can disable automated security fix PRs for this repo from the
[Security Alerts
page](https://github.com/launchdarkly/vue-client-sdk-private/network/alerts).

</details>

> **Note**
> Automatic rebases have been disabled on this pull request as it has
been open for over 30 days.
@rich-field
Copy link

This seems like a particularly powerful and useful feature to have, any update on this?

@kinyoklion kinyoklion requested a review from a team as a code owner August 12, 2024 21:00
/**
* Will hold the LaunchDarkly instance.
*/
export const launchDarklyClient = ref() as Ref<LDClient>
Copy link
Member

Choose a reason for hiding this comment

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

I think this breaks scoping/encapsulation. As this ref would be the same even with two apps both using the LDPlugin.

Where the provide/inject for the built-in ready and client theoretically would be scoped.

I think that route guards support inject as of vue 3.3. https://router.vuejs.org/guide/advanced/navigation-guards.html#Global-injections-within-guards

I need to look into what the implications of us upgrading would be.

I think, in that case, useLDClient and useLDReady could be used without duplicating these refs and without causing a scope problem.

@yusinto yusinto removed their request for review August 29, 2024 16:40
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.

8 participants