@W-17701711@ Generate isomorphic SDK from OAS by vcua-mobify · Pull Request #194 · SalesforceCommerceCloud/commerce-sdk-isomorphic

vcua-mobify · 2025-05-23T22:29:32Z

THIS PR CONTAINS BREAKING CHANGES

This PR updates commerce-sdk-isomorphic so that it generates from OAS spec files rather than RAML files.

This requires the changes from SalesforceCommerceCloud/raml-toolkit#228 to work

Testing the changes:

Check out both this PR and the raml-toolkit pr
Build raml-toolkit: npm install && npm run build
In commerce-sdk-isomorphic's package.json, ensure the dependency to raml-toolkit is pointed to your local raml-toolkit.
Run yarn ci to install dependencies and include the local raml-toolkit
Set some env vars for the anypoint exchange credentials
export ANYPOINT_USERNAME="<insert_username_here>"
export ANYPOINT_PASSWORD="<insert_password_here>"
Download the OAS files from anypoint exchange and run diff check. You currently need to run this twice; the 1st run will compare the current contents of the apis dir (which contain raml files) with the newly downloaded oas files. The 2nd run onwards will compare only oas files (the files you downloaded from the 1st run with the files you downloaded from the 2nd run)
yarn updateApis
yarn renderTemplates will call raml-toolkit to run openapi-generator. The files it generates will be outputted under src/lib
Currently the generated files have a naming conflict for ShopperContext. Current workaround is to manually rename the ShopperContext class to ShopperContexts and update the corresponding entry in src/lib/index.ts
Bundle the generated files with yarn build:lib
Run tests with yarn test

TODO - will be done in a follow up PR:
* Identify why the bundled package is 1 mb (double the previous size of 565 kb). Once resolved, bump down the threshold for bundle size in package.json

UPDATE: Only the .ts files have doubled in size. The minified .js files are roughly the same size so we might be ok with this.
Fix issues in the generated code
- Work with API teams to resolve ShopperContext naming conflict
- Resolve duplicate naming issue with types in the bundled code from step 7
  * Commit the OAS files / replace the RAML files in the apis dir - see https://github.com/SalesforceCommerceCloud/commerce-sdk-isomorphic/pull/195/files This is done
Major version bump

Out of Scope
The test app / yarn start as it seems to have been broken for a while

package.json

unandyala · 2025-05-27T18:21:18Z

scripts/config.ts

-  'shopper-baskets',
-  'shopper-context',
-  'shopper-customers',
-  'shopper-discovery-search',


is shopper-discovery-search no longer needed?

Yes, according to Kay, both shopper-discovery-search and slas-shopper-login-uap artifact are out of scope for the OAS work

I assume we are not using these in PWA?

unandyala · 2025-05-27T20:18:15Z

scripts/generate-oas.ts

+}
+
+export function main(): void {
+  console.log('Starting OAS generation script');


Can you separate this (until line #120 )into its own function - loadApiDirectories

Everything from line 116 to 141 is inside the callback function passed into fs.readdir
Outside of that, there isn't much; just a ternary for determining the API directory and 2 lines for copying the static directory. I can separate out the static file bits.

unandyala · 2025-05-27T20:33:07Z

scripts/updateApis.ts

 fs.ensureDirSync(PRODUCTION_API_PATH);

 API_LIST.forEach(name => {
+  // TODO: come up with a way to set isOAS other than using the name


If we really want to maintain code for OAS and RAML then create a new file for OAS - updateOpenApis.ts. But I feel if we are getting rid RAML completely then default to OAS.

Question- Does the generated SDK from OAS has same interface has the RAML?

It appears to have the same interface from a glance but I have not done a deep dive yet

isOAS seems unnecessary as we only support OAS in the SDK. Can we default to true while calling the raml-toolkit?

unandyala · 2025-05-27T20:33:51Z

scripts/utils.ts

  name: string,
-  rootPath: string
+  rootPath: string,
+  isOAS = false


same here. Do we really need this if OAS is the only spec

src/environment/App/App.jsx

unandyala · 2025-05-27T20:39:41Z

src/static/helpers/slasHelper.ts

  const privateClient = !!credentials.clientSecret;

-  const tokenBody: TokenRequest = {
+  const tokenBody = {


We removed the type because we are not able to generate the type for the request body?

Yes, the removal of TokenRequest is one of the things called out by @joeluong-sfcc in his spike: https://github.com/joeluong-sfcc/oas-spike/pull/11

src/static/version.ts

templatesRaml/index.ts.hbs

joeluong-sfcc · 2025-05-29T13:53:45Z

package.json

    "@typescript-eslint/parser": "^4.33.0",
    "autoprefixer": "9.8.8",
    "bundlesize2": "^0.0.31",
+    "cross-env": "^5.2.1",


I believe we use cross-env in our other projects, but just double checking that this is 3PP approved correct?

This is the exact same version of cross-env used by the PWA Kit. But I'll remove this since I added this as a just in case someone were to run this in windows, which is admittedly unlikely.

joeluong-sfcc · 2025-05-29T14:00:15Z

package.json

    {
      "path": "commerce-sdk-isomorphic-with-deps.tgz",
-      "maxSize": "565 kB"
+      "maxSize": "2048 kB"


I was expecting this to jump but not by that much, but to be honest, I think this should be fine. The bundle size that we really care about is the one that gets sent to the browser, which is the minified version generated by rollup in ./lib.

We still have to update the rollup config to split up files to support tree-shaking, and so we won't exactly know how the minified bundle size will be affected until then

The actual size of the tgz is 1.1 mb, and is caused by the .ts files doubling in size. The minified files have grown by less than 1 kb.

I'll lower the max size to something like 1.2 mb for now

joeluong-sfcc · 2025-05-29T18:39:23Z

package.json

    "lint:style": "stylelint ./src/",
    "prepare": "snyk protect",
-    "renderTemplates": "ts-node --compiler-options '{\"module\": \"commonjs\", \"target\": \"ES6\" }' ./scripts/generate.ts",
+    "renderTemplates": "cross-env PACKAGE_VERSION=$(node -p \"require('./package.json').version\") ts-node --compiler-options '{\"module\": \"commonjs\", \"target\": \"ES6\" }' ./scripts/generate-oas.ts",


What's the use case for using cross-env?

It was just a just in case someone were to run this in windows since we're setting an env var before running the generator. But as that is unlikely, I'll remove it for now.

vcua-mobify · 2025-05-29T21:14:58Z

scripts/generate.ts

This is the generate from raml script. Removed since the isomorphic sdk will only be generated from oas.

vcua-mobify · 2025-05-29T21:16:29Z

scripts/utils.ts

@@ -175,18 +36,21 @@ export async function updateApis(
 */
 export async function downloadLatestApis(


This is the only method in utils.ts that is used by the oas version of updateApis. Every other function here has been removed as they were only used by the raml generator.

unandyala · 2025-05-29T21:23:30Z

scripts/config.ts

-  'shopper-baskets',
-  'shopper-context',
-  'shopper-customers',
-  'shopper-discovery-search',


I assume we are not using these in PWA?

unandyala · 2025-05-29T21:27:57Z

src/test/parameters.test.ts


 import nock from 'nock';
-import {ShopperSearch} from '../lib/shopperSearch';
+import {ShopperSearch} from '../lib/shopper-search';


Are we planning to change the case in a different PR?

I think the imports here are ok since they only affect the non-bundled APIs.

In the bundled code, the imports should still be the same. Ie.
import {ShopperSearch} from 'commerce-sdk-isomorphic'

unandyala · 2025-05-29T21:44:48Z

scripts/updateApis.ts

 fs.ensureDirSync(PRODUCTION_API_PATH);

 API_LIST.forEach(name => {
+  // TODO: come up with a way to set isOAS other than using the name


isOAS seems unnecessary as we only support OAS in the SDK. Can we default to true while calling the raml-toolkit?

vcua-mobify · 2025-05-29T21:58:30Z

@unandyala the entries in the config file correspond to the names of the files in anypoint exchange.
The actual APIs themselves are mostly the same. I say mostly because discovery in particular is out of scope.

From what I can see, we don't use discovery in commerce-sdk-react

joeluong-sfcc · 2025-05-30T14:56:20Z

package.json

    "@babel/preset-react": "7.18.6",
    "@babel/preset-typescript": "^7.18.6",
-    "@commerce-apps/raml-toolkit": "0.5.12",
+    "@commerce-apps/raml-toolkit": "file:../raml-toolkit",


Will we wait until the RAML toolkit is released and then update this before merging? If not, let's note somewhere that we have to update this so we don't forget

I've updated this to the newly released raml-toolkit version

joeluong-sfcc · 2025-05-30T14:57:06Z

scripts/config.ts

-  'shopper-search',
-  'shopper-seo',
-  'shopper-stores',
+  'shopper-baskets-oas',


I left a comment in the other PR for just replacing the APIs, but do we want to add shopper baskets v1 as well?

Looking at anypoint exchange, there is only one version of shopper-baskets-oas.

We should bring up getting an OAS version of baskets v1 with the api team.

joeluong-sfcc · 2025-05-30T15:07:35Z

src/test/requests.test.ts

+  ShopperLogin,
+} from '../lib/shopper-login';
+import ClientConfig, {ClientConfigInit} from '../static/clientConfig';
+import {ShopperBaskets} from '../lib/shopper-baskets';


[NIT] but if we have time let's update the generator to use camelCase instead of kebab-case so it's ../lib/shopperBaskets just for consistency

This suggestion has been applied

joeluong-sfcc · 2025-05-30T15:11:42Z

src/static/version.ts

+ * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
+ */
+export const USER_AGENT_HEADER = "user-agent";
+export const USER_AGENT_VALUE = "commerce-sdk-isomorphic@3.3.0";


This file is generated correct? Do we need this file since we already generate one with the SDK in src/lib/version.ts?

Good eye. This is a current quirk where the generated version of version.ts outputs to /src/static then is copied over into src/lib later on.

The reason I had this is because the templates currently set imports to go to the static directory rather than the lib directory.

I'll update the templates to point to /src/lib rather than src/static then we won't need this file anymore

joeluong-sfcc · 2025-05-30T15:13:55Z

src/static/helpers/slasHelper.ts

    },
    body: {
-      grant_type: 'client_credentials',
+      grant_type: 'client_credentials' as const,


Was this added because there were new typescript errors?

That's correct, yes. There were errors around the 'client_credentials' string not being allowed with the enum values of grant_type without this.

joeluong-sfcc · 2025-05-30T15:17:10Z

scripts/generate-oas.ts

+  organizationId: string;
+  name: string;
+  tags: string[];
+};


Does this type already exist in RAML toolkit? If so, we should export it there and import it here instead

This type does not exist in raml toolkit but I could move it there if we want since I'm duplicating this type in the node sdk

This type is now in the raml toolkit and this PR has been updated to refer there

unandyala

left small comment/question

unandyala · 2025-06-02T19:56:48Z

openapitools.json

@@ -0,0 +1,7 @@
+{


Do we need this file here? Will it not use from raml-toolkit?

Good question. I just tried generating the sdk again using the raml-toolkit command after deleting it and it didn't return.
This was probably an artifact from when I ran openapi-generator directly in the sdk. I'll delete it.

vcua-mobify added 16 commits May 14, 2025 16:52

Add oas generator

bdb11d4

Add oas templates

1176306

sort package.json

1fd3f58

Add oas specific npm scripts

1568fc6

Set OAS templates

dca3c90

Copy static dir and update imports

adf102f

Update test imports

1e296d6

Add version file to static

34ece5e

New template for index file

45aa643

Fix issues with the api template and types

35894e1

REVERT THIS: remove commit hook so I can actually push a feature branch

c0b4578

Remove hasImports from oneOf

3a9082b

Re-add hooks

6aeb85e

Fix tests

1a3f6f8

Revisit this: adjust thresholds

d6ac769

Fix tests and restore test thresholds

1fa97bd

vcua-mobify changed the base branch from main to feature/oas May 27, 2025 16:36

unandyala reviewed May 27, 2025

View reviewed changes

vcua-mobify added 3 commits May 28, 2025 13:22

Revert app change

ed22fe5

Separate copyStatic

709f8b4

Dynamic version

5f9e3e5

vcua-mobify requested a review from unandyala May 28, 2025 22:09

Replace raml commands with oas in package.json

7e8685c

vcua-mobify requested a review from joeluong-sfcc May 29, 2025 17:16

joeluong-sfcc reviewed May 29, 2025

View reviewed changes

vcua-mobify added 2 commits May 29, 2025 13:47

Remove cross-env

75ec28f

Delete raml-specific templates and scripts

00b774c

vcua-mobify requested a review from joeluong-sfcc May 29, 2025 21:13

vcua-mobify commented May 29, 2025

View reviewed changes

unandyala reviewed May 29, 2025

View reviewed changes

Default isOAS to true when downloading from exchange

ab5160e

unandyala approved these changes May 30, 2025

View reviewed changes

joeluong-sfcc reviewed May 30, 2025

View reviewed changes

vcua-mobify added 7 commits June 2, 2025 10:11

Use ExchangeConfig type from raml-toolkit

a7db4ee

Refer to contents inside lib instead of static dir

bbbb255

Use camel case instead of kebab case and fix tests

2813adc

Update test imports

726d8ff

More test import updates

8aa5b6f

whitespace

de5f943

Use new raml-toolkit release

98adab1

vcua-mobify changed the title ~~@W-17701711@ WIP - DO NOT MERGE - Generate isomorphic SDK from OAS~~ @W-17701711@ Generate isomorphic SDK from OAS Jun 2, 2025

vcua-mobify marked this pull request as ready for review June 2, 2025 19:48

vcua-mobify requested a review from a team as a code owner June 2, 2025 19:48

unandyala approved these changes Jun 2, 2025

View reviewed changes

Delete openapitools.json

d4d34cd

joeluong-sfcc approved these changes Jun 3, 2025

View reviewed changes

vcua-mobify merged commit e54456c into feature/oas Jun 3, 2025
4 of 10 checks passed

		@@ -175,18 +36,21 @@ export async function updateApis(
		*/
		export async function downloadLatestApis(

Conversation

vcua-mobify commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vcua-mobify May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vcua-mobify commented May 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vcua-mobify commented May 23, 2025 •

edited

Loading

vcua-mobify May 29, 2025 •

edited

Loading