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.

Sign up for GitHub

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

Jump to bottom

Remove future flag for released feature #1705

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+65 −110
Open

Remove future flag for released feature #1705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
870926b
Remove future flag for optional scopes
zzooeeyy Oct 29, 2024
aa4e9ff
Add changeset
zzooeeyy Oct 29, 2024
f1d984d
Regenerate docs
zzooeeyy Oct 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/four-bugs-fix.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/shopify-app-remix': minor
---

Add Scopes API to Remix library by removing the future flag that enabled this feature
132 changes: 52 additions & 80 deletions packages/apps/shopify-app-remix/docs/generated/generated_docs_data.json
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion packages/apps/shopify-app-remix/src/server/__test-helpers/test-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import type {FutureFlags, FutureFlagOptions} from '../future/flags';
*/
const TEST_FUTURE_FLAGS: Required<{[key in keyof FutureFlags]: true}> = {
unstable_newEmbeddedAuthStrategy: true,
wip_optionalScopesApi: true,
} as const;

// Override the helper's future flags and logger settings for our purposes
Expand Down
11 changes: 4 additions & 7 deletions packages/apps/shopify-app-remix/src/server/authenticate/admin/authenticate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,13 +132,10 @@ export function authStrategyFactory<
}

function addScopesFeatures(context: AdminContextBase) {
if (config.future.wip_optionalScopesApi) {
return {
...context,
scopes: scopesApiFactory(params, context.session, context.admin),
};
}
return context;
return {
...context,
scopes: scopesApiFactory(params, context.session, context.admin),
};
}

return async function authenticateAdmin(request: Request) {
Expand Down
6 changes: 3 additions & 3 deletions ...ages/apps/shopify-app-remix/src/server/authenticate/admin/scope/__tests__/request.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ import * as redirect from '../../helpers/redirect-to-install-page';

import * as responses from './mock-responses';

it('when the future flag is disabled the scopes api is not available', async () => {
it('scopes api is available without any future flags', async () => {
// GIVEN
const shopify = shopifyApp(
testConfig({
isEmbeddedApp: false,
scopes: undefined,
future: {wip_optionalScopesApi: false},
future: {},
}),
);
const session = await setUpValidSession(shopify.sessionStorage);
Expand All @@ -39,7 +39,7 @@ it('when the future flag is disabled the scopes api is not available', async ()
const adminApi = await shopify.authenticate.admin(request);

// THEN
expect(adminApi).not.toHaveProperty('scopes');
expect(adminApi).toHaveProperty('scopes');
});

it('when scopes are empty the request is not redirected', async () => {
Expand Down
6 changes: 1 addition & 5 deletions packages/apps/shopify-app-remix/src/server/authenticate/admin/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import {JwtPayload, Session, ShopifyRestResources} from '@shopify/shopify-api';
import {EnsureCORSFunction} from '../helpers/ensure-cors-headers';
import type {AppConfigArg} from '../../config-types';
import type {AdminApiContext} from '../../clients';
import {FeatureEnabled} from '../../future/flags';

import type {BillingContext} from './billing/types';
import {RedirectFunction} from './helpers/redirect';
Expand Down Expand Up @@ -205,10 +204,7 @@ export interface ScopesContext {
export type AdminContext<
Config extends AppConfigArg,
Resources extends ShopifyRestResources,
> =
FeatureEnabled<Config['future'], 'wip_optionalScopesApi'> extends true
? EmbeddedTypedAdminContext<Config, Resources> & ScopesContext
: EmbeddedTypedAdminContext<Config, Resources>;
> = EmbeddedTypedAdminContext<Config, Resources> & ScopesContext;

export type AuthenticateAdmin<
Config extends AppConfigArg,
Expand Down
14 changes: 0 additions & 14 deletions packages/apps/shopify-app-remix/src/server/future/flags.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,6 @@ export interface FutureFlags {
* @default false
*/
unstable_newEmbeddedAuthStrategy?: boolean;

/**
* When enabled, the Scopes API will be available. This feature is in development and requires special permissions from Shopify for now.
*
* @default false
*/
wip_optionalScopesApi?: boolean;
}

// When adding new flags, use this format:
Expand Down Expand Up @@ -63,11 +56,4 @@ export function logDisabledFutureFlags(
'\n Your app must be using Shopify managed install: https://shopify.dev/docs/apps/auth/installation',
);
}

if (!config.future.wip_optionalScopesApi) {
logFlag(
'wip_optionalScopesApi',
'Enable this to use the optionalScopes API to request additional scopes and manage them. ',
);
}
}
Loading