Skip to content

Fix: Isomorphic OAS SDK gaps (@W-18669767@) #197

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

Merged
joeluong-sfcc merged 13 commits into from
Jun 11, 2025
Merged

Fix: Isomorphic OAS SDK gaps (@W-18669767@) #197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
39d2a0a
update APIs
joeluong-sfcc Jun 3, 2025
8cba0b4
fix naming collisions
joeluong-sfcc Jun 3, 2025
1307373
add fix for shopper login
joeluong-sfcc Jun 3, 2025
3488afa
Merge branch 'feature/oas' into ju/fix-enum-collision
joeluong-sfcc Jun 4, 2025
18af95c
add custom classname support
joeluong-sfcc Jun 4, 2025
31b5893
resolve failing tests and lint some code
joeluong-sfcc Jun 5, 2025
2add33f
fix more linting errors
joeluong-sfcc Jun 5, 2025
ac541c4
fix linting errors and failing tests
joeluong-sfcc Jun 5, 2025
dc93377
remove node support for 12, 14, 16, and 18
joeluong-sfcc Jun 5, 2025
7ac7938
use x-sdk-classname
joeluong-sfcc Jun 10, 2025
c14441c
resolve shopper login naming collision issue in a cleaner way
joeluong-sfcc Jun 10, 2025
648cbc4
fix tests
joeluong-sfcc Jun 10, 2025
6cfdce0
add openapitools.json to .gitignore
joeluong-sfcc Jun 10, 2025
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: 3 additions & 2 deletions .eslintrc.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,10 @@
}
},
{
"files": ["./src/static/helpers/slasHelper.ts"],
"files": ["./src/static/helpers/slasHelper.ts", "./src/static/helpers/types.ts"],
"rules": {
"max-lines": "off"
"max-lines": "off",
"camelcase": "off"
}
}
],
Expand Down
3 changes: 1 addition & 2 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ jobs:
linux-tests:
strategy:
matrix:
# TODO: remove older versions of node
node: [12, 14, 16, 18, 20, 22]
node: [20, 22]
fail-fast: false
runs-on: ubuntu-latest
steps:
Expand Down
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# build files
/src/lib
src/react-app-env.d.ts
openapitools.json

# dependencies
/node_modules
Expand Down
3 changes: 1 addition & 2 deletions apis/shopper-context-oas/shopper-context-oas-v1-bundled.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ servers:
variables:
shortCode:
default: shortCode
x-sdk-classname: ShopperContexts
paths:
/organizations/{organizationId}/shopper-context/{usid}:
get:
Expand Down Expand Up @@ -538,5 +539,3 @@ components:
regionCode: '12345'
latitude: 10.11
longitude: 198.34
x-sdk-classname:
classname: ShopperContexts
7 changes: 6 additions & 1 deletion scripts/generate-oas.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ const VERSION_TEMPLATE_LOCATION = path.join(
);

function kebabToCamelCase(str: string): string {
return str.replace(/-([a-z])/g, (match, letter: string) => letter.toUpperCase());
return str.replace(/-([a-z])/g, (match, letter: string) =>
letter.toUpperCase()
);
}

export function resolveApiName(name: string): string {
Expand All @@ -43,6 +45,9 @@ export function resolveApiName(name: string): string {
if (name === 'Shopper Seo OAS') {
return 'ShopperSEO';
}
if (name === 'Shopper Context OAS') {
return 'ShopperContexts';
}
return name.replace(/\s+/g, '').replace('OAS', '');
}

Expand Down
8 changes: 5 additions & 3 deletions src/static/helpers/slasHelper.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,10 +233,13 @@

// There should be no code_challenge for private client
const expectedReqOptions = {
accessToken: "access_token",

Check warning on line 236 in src/static/helpers/slasHelper.test.ts

View workflow job for this annotation

GitHub Actions / linux-tests (22) 

Replace `"access_token"` with `'access_token'`

Check warning on line 236 in src/static/helpers/slasHelper.test.ts

View workflow job for this annotation

GitHub Actions / linux-tests (20) 

Replace `"access_token"` with `'access_token'`
Copy link
Contributor

@vcua-mobify vcua-mobify Jun 6, 2025

Choose a reason for hiding this comment

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

heads up that I applied a fix to this test also as part of #198 .

In there, I replaced expectedReqOptions with a check that ensures code_challenge wasn't included.

client_id: 'client_id',
channel_id: 'site_id',
dnt: "false",

Check warning on line 239 in src/static/helpers/slasHelper.test.ts

View workflow job for this annotation

GitHub Actions / linux-tests (22) 

Replace `"false"` with `'false'`

Check warning on line 239 in src/static/helpers/slasHelper.test.ts

View workflow job for this annotation

GitHub Actions / linux-tests (20) 

Replace `"false"` with `'false'`
hint: 'hint',
redirect_uri: 'redirect_uri',
refreshToken: 'refresh_token',
response_type: 'code',
usid: 'usid',
};
Expand Down Expand Up @@ -381,7 +384,7 @@
expect(accessToken).toBe(expectedTokenResponse);
});

test('throw warning for invalid params', async () => {
test('throws warning for invalid params', async () => {
// Spy on console.warn
const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation();

Expand Down Expand Up @@ -415,7 +418,7 @@

// Assert the warning was logged
expect(consoleWarnSpy).toHaveBeenCalledWith(
expect.stringContaining('Invalid Parameter for authorizeCustomer: hello')
expect.stringContaining('Found unknown parameter for authorizeCustomer: hello, adding as query parameter anyway')

Check warning on line 421 in src/static/helpers/slasHelper.test.ts

View workflow job for this annotation

GitHub Actions / linux-tests (22) 

Replace `'Found·unknown·parameter·for·authorizeCustomer:·hello,·adding·as·query·parameter·anyway'` with `⏎········'Found·unknown·parameter·for·authorizeCustomer:·hello,·adding·as·query·parameter·anyway'⏎······`

Check warning on line 421 in src/static/helpers/slasHelper.test.ts

View workflow job for this annotation

GitHub Actions / linux-tests (20) 

Replace `'Found·unknown·parameter·for·authorizeCustomer:·hello,·adding·as·query·parameter·anyway'` with `⏎········'Found·unknown·parameter·for·authorizeCustomer:·hello,·adding·as·query·parameter·anyway'⏎······`
);

expect(getAccessTokenMock).toBeCalledWith(expectedTokenBody);
Expand All @@ -424,7 +427,6 @@
// Restore the original console.warn
consoleWarnSpy.mockRestore();
});

test('can pass custom params on public guest', async () => {
const expectedTokenBody = {
body: {
Expand Down
4 changes: 2 additions & 2 deletions src/static/helpers/slasHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import {
ShopperLogin,
ShopperLoginPathParameters,
ShopperLoginQueryParameters,
TokenResponse,
} from '../../lib/shopperLogin';
} from '../../lib/shopperLogin/apis';
import {TokenResponse} from '../../lib/shopperLogin/models';
Copy link
Contributor

@vcua-mobify vcua-mobify Jun 10, 2025

Choose a reason for hiding this comment

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

We went over this in the meet earlier but could you restate why this change in particular is needed or how the original version resulted in a conflict in the rolled up typescript?

Copy link
Contributor Author

@joeluong-sfcc joeluong-sfcc Jun 11, 2025

Choose a reason for hiding this comment

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

Yeah, when we imported directly from the top level index file, it would duplicate the namespace ShopperLoginTypes in the generated *.d.ts file

By importing from the apis/models directory directly, we circumvent this issue

import ResponseError from '../responseError';
import TemplateURL from '../templateUrl';
import {
Expand Down
2 changes: 0 additions & 2 deletions src/test/parameters.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ describe('Parameters', () => {
);

it('has a list of parameter keys (and the required ones)', () => {
// eslint-disable-next-line
expect(ShopperSearch.paramKeys?.productSearch).toBeDefined();
// eslint-disable-next-line
expect(ShopperSearch.paramKeys?.productSearchRequired).toBeDefined();
});

Expand Down
9 changes: 7 additions & 2 deletions src/test/requests.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,14 @@ import nock from 'nock';
import {BaseUriParameters} from 'lib/helpers';
import {
ShopperLoginPathParameters,
ShopperLoginQueryParameters
ShopperLoginQueryParameters,
} from '../lib/shopperLogin';
import {ClientConfig, ClientConfigInit, ShopperLogin, ShopperBaskets} from '../lib';
import {
ClientConfig,
ClientConfigInit,
ShopperLogin,
ShopperBaskets,
} from '../lib';

const config: ClientConfigInit<
ShopperLoginPathParameters &
Expand Down
2 changes: 1 addition & 1 deletion templates/index.ts.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{{#each children}}
export { {{apiName}} } from "./{{directoryName}}";
import type * as {{apiName}}Types from "./{{directoryName}}";
import { {{apiName}}Types } from "./{{directoryName}}";
export type { {{apiName}}Types };
{{/each}}
export type { ClientConfigInit } from "./clientConfig";
Expand Down
2 changes: 1 addition & 1 deletion templatesOas/apis.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ export type {{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camel
*
*
*/
export class {{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}<ConfigParameters extends {{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}Parameters & Record<string, unknown>> {
export class {{#vendorExtensions}}{{#x-sdk-classname}}{{{ . }}}{{/x-sdk-classname}}{{^x-sdk-classname}}{{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}{{/x-sdk-classname}}{{/vendorExtensions}}{{^vendorExtensions}}{{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}{{/vendorExtensions}}<ConfigParameters extends {{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}Parameters & Record<string, unknown>> {
Copy link
Contributor

@vcua-mobify vcua-mobify Jun 10, 2025

Choose a reason for hiding this comment

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

Thanks for this; I've applied the template changes (with some slight tweaks) to the node SDK PR: SalesforceCommerceCloud/commerce-sdk#418

The tweaks were to account for the namespacing that we do in the node sdk.

In the future I wonder if there is some way we can make this easier to read or some way to extract it so we only define the full vendorExtensions block once. Not a blocker for this PR though.

// baseUri is not required on ClientConfig, but we know that we provide one in the class constructor
public clientConfig: ClientConfig<ConfigParameters> & { baseUri: string };

Expand Down
20 changes: 10 additions & 10 deletions templatesOas/index.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,26 +8,26 @@ export * from './apis/index{{importFileExtension}}';
export * from './models/index{{importFileExtension}}';
{{/models.0}}

import type * as ApiTypes from './apis/index'
import type * as ModelTypes from './models/index'
import type * as {{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}ApiTypes from './apis/index'
import type * as {{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}ModelTypes from './models/index'

export namespace {{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}Types {
export namespace {{#apiInfo}}{{#apis.0}}{{#vendorExtensions}}{{#x-sdk-classname}}{{{ . }}}{{/x-sdk-classname}}{{^x-sdk-classname}}{{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}{{/x-sdk-classname}}{{/vendorExtensions}}{{^vendorExtensions}}{{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}{{/vendorExtensions}}{{/apis.0}}{{/apiInfo}}Types {
// API types
export type {{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}PathParameters = ApiTypes.{{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}PathParameters
export type {{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}QueryParameters = ApiTypes.{{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}QueryParameters
export type {{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}PathParameters = {{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}ApiTypes.{{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}PathParameters
export type {{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}QueryParameters = {{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}ApiTypes.{{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}QueryParameters
{{#apiInfo}}
{{#apis}}
{{#operations}}
{{#operation}}
{{#allParams}}
{{#isEnum}}
{{^stringEnums}}
export type {{operationIdCamelCase}}{{enumName}} = ApiTypes.{{operationIdCamelCase}}{{enumName}}
export type {{operationIdCamelCase}}{{enumName}} = {{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}ApiTypes.{{operationIdCamelCase}}{{enumName}}
{{/stringEnums}}
{{/isEnum}}
{{/allParams}}
export type {{nickname}}QueryParameters = ApiTypes.{{nickname}}QueryParameters
export type {{nickname}}PathParameters = ApiTypes.{{nickname}}PathParameters
export type {{nickname}}QueryParameters = {{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}ApiTypes.{{nickname}}QueryParameters
export type {{nickname}}PathParameters = {{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}ApiTypes.{{nickname}}PathParameters
{{/operation}}
{{/operations}}
{{/apis}}
Expand All @@ -36,11 +36,11 @@ export namespace {{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.
// Model types
{{#models}}
{{#model}}
export type {{classname}} = ModelTypes.{{classname}}
export type {{classname}} = {{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}ModelTypes.{{classname}}
{{#hasEnums}}
{{#vars}}
{{#isEnum}}
export type {{classname}}{{enumName}} = ModelTypes.{{classname}}{{enumName}}
export type {{classname}}{{enumName}} = {{#lambda.titlecase}}{{#lambda.camelcase}}{{appName}}{{/lambda.camelcase}}{{/lambda.titlecase}}ModelTypes.{{classname}}{{enumName}}
{{/isEnum}}
{{/vars}}
{{/hasEnums}}
Expand Down
11 changes: 3 additions & 8 deletions templatesOas/modelEnumInterfaces.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
{{! *** Default Implementation from Open API generator }}
{{#stringEnums}}
/**
* {{#lambda.indented_star_1}}{{{unescapedDescription}}}{{/lambda.indented_star_1}}
Expand All @@ -20,17 +19,13 @@ export enum {{classname}} {
/**
* {{#lambda.indented_star_1}}{{{unescapedDescription}}}{{/lambda.indented_star_1}}
*/
export const {{classname}} = {
export type {{classname}} =
{{#allowableValues}}
{{#enumVars}}
{{#enumDescription}}
/**
* {{enumDescription}}
*/
// {{enumDescription}}
{{/enumDescription}}
{{{name}}}: {{{value}}}{{^-last}},{{/-last}}
{{{value}}}{{^-last}}|{{/-last}}{{#-last}};{{/-last}}
{{/enumVars}}
{{/allowableValues}}
} as const;
export type {{classname}} = typeof {{classname}}[keyof typeof {{classname}}];
{{/stringEnums}}
6 changes: 2 additions & 4 deletions templatesOas/modelGenericInterfaces.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,12 @@ export enum {{classname}}{{enumName}} {
{{/allowableValues}}
}
{{/stringEnums}}{{^stringEnums}}
Copy link
Contributor Author

@joeluong-sfcc joeluong-sfcc Jun 6, 2025

Choose a reason for hiding this comment

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

The resulting type is functionally the same, we just remove the exported variable. Example:

Before:

export const BasketChannelTypeEnum = {
    Storefront: 'storefront',
    Callcenter: 'callcenter',
    Marketplace: 'marketplace',
    Dss: 'dss',
    Store: 'store',
    Pinterest: 'pinterest',
    Twitter: 'twitter',
    Facebookads: 'facebookads',
    Subscriptions: 'subscriptions',
    Onlinereservation: 'onlinereservation',
    Customerservicecenter: 'customerservicecenter',
    Instagramcommerce: 'instagramcommerce',
    Tiktok: 'tiktok',
    Snapchat: 'snapchat',
    Google: 'google',
    Whatsapp: 'whatsapp',
    Youtube: 'youtube'
} as const;

// type BasketChannelTypeEnum = "storefront" | "callcenter" | "marketplace" | "dss" | "store" | "pinterest" | "twitter" | "facebookads" | "subscriptions" | "onlinereservation" | "customerservicecenter" | ... 5 more ... | "youtube" 
export type BasketChannelTypeEnum = typeof BasketChannelTypeEnum[keyof typeof BasketChannelTypeEnum];

After:

export type BasketChannelTypeEnum = 
    'storefront'|
    'callcenter'|
    'marketplace'|
    'dss'|
    'store'|
    'pinterest'|
    'twitter'|
    'facebookads'|
    'subscriptions'|
    'onlinereservation'|
    'customerservicecenter'|
    'instagramcommerce'|
    'tiktok'|
    'snapchat'|
    'google'|
    'whatsapp'|
    'youtube';

export const {{classname}}{{enumName}} = {
export type {{classname}}{{enumName}} =
{{#allowableValues}}
{{#enumVars}}
{{{name}}}: {{{value}}}{{^-last}},{{/-last}}
{{{value}}}{{^-last}}|{{/-last}}{{#-last}};{{/-last}}
{{/enumVars}}
{{/allowableValues}}
} as const;
export type {{classname}}{{enumName}} = typeof {{classname}}{{enumName}}[keyof typeof {{classname}}{{enumName}}];
{{/stringEnums}}
{{/isEnum}}
{{/vars}}
Expand Down
Loading