refactor: migrate `mc-scripts` package to TypeScript #2613

emmenko · 2022-05-13T20:22:57Z

Following up some previous work #2578 and #2579

See inline comments for more information.

In general, I wanted to bundle the CLI using Preconstruct. Right now we use Babel and we produce the same files compiled, mostly because we rely on file system paths.
However, this PR changes that and we leverage code splitting to load the required chunk.

TODO:

changeset-bot · 2022-05-13T20:23:00Z

🦋 Changeset detected

Latest commit: d184a24

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages

Name	Type
@commercetools-frontend/mc-scripts	Minor
@commercetools-frontend/assets	Minor
merchant-center-application-template-starter	Patch
playground	Patch
@commercetools-frontend/application-components	Minor
@commercetools-frontend/application-shell	Minor
@commercetools-local/visual-testing-app	Patch
@commercetools-website/components-playground	Patch
@commercetools-frontend/cypress	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

emmenko · 2022-05-13T20:24:23Z

packages/mc-scripts/package.json

+  "preconstruct": {
+    "entrypoints": ["./index.ts", "./application-runtime.ts"]
+  },


The application-runtime is a separate entry point so that the file can be referenced from the web pack config.

emmenko · 2022-05-13T20:25:25Z

packages/mc-scripts/package.json

@@ -42,6 +54,7 @@
    "autoprefixer": "^10.4.4",
    "babel-loader": "8.2.5",
    "browserslist": "^4.20.2",
+    "cac": "6.7.12",


New CLI library as a replacement for mri, as it has some nicer features.

https://www.npmjs.com/package/cac

emmenko · 2022-05-13T20:28:03Z

packages/mc-scripts/src/bin/cli.ts

+      const startCommand = shouldUseExperimentalBundler
+        ? await import('../commands/start-vite')
+        : await import('../commands/start');
+      await startCommand.default();


This is now how we load and execute the commands. The bundle then contains code-splitter chunks.

emmenko · 2022-05-13T20:28:45Z

packages/mc-scripts/src/bin/cli.ts

+// Load dotenv files into the process environment.
+// This is essentially what `dotenv-cli` does, but it's now built into this CLI.
+// Inspired also by https://create-react-app.dev/docs/adding-custom-environment-variables/#what-other-env-files-can-be-used
+function loadDotEnvFiles(globalOptions: TCliGlobalOptions) {


Nothing changed here

emmenko · 2022-05-13T20:29:47Z

packages/mc-scripts/src/commands/build.ts

+const WARN_AFTER_BUNDLE_GZIP_SIZE = 512 * 1024;
+const WARN_AFTER_CHUNK_GZIP_SIZE = 1024 * 1024;
+
+async function run() {


Nothing much changed here, besides wrapping the logic into the run function and using more async/await

emmenko · 2022-05-13T20:30:14Z

packages/mc-scripts/src/commands/compile-html.ts

+
+const appDirectory = fs.realpathSync(process.cwd());
+
+async function run(options: TCliCommandCompileHtmlOptions = {}) {


Nothing changed here

emmenko · 2022-05-13T20:30:30Z

packages/mc-scripts/src/commands/serve.ts

+
+const port = 3001;
+
+async function run() {


Nothing changed here

emmenko · 2022-05-13T20:32:12Z

packages/mc-scripts/src/commands/start.ts

+import createDevServerConfig from '../config/webpack-dev-server.config';
+import createWebpackConfigForDevelopment from '../config/create-webpack-config-for-development';
+
+async function run() {


Nothing much changed here, besides wrapping into run function, using async/await and cleaning up a bit.

github-actions · 2022-05-13T20:32:21Z

Deploy preview for merchant-center-application-kit ready!

✅ Preview
https://merchant-center-application-nlizjbn80-commercetools.vercel.app
https://appkit-sha-c23d08f79dde28221aee518a90be143b9e3e9455.commercetools.vercel.app
https://appkit-pr-2613.commercetools.vercel.app

Built with commit d184a24.
This pull request is being automatically deployed with vercel-action

emmenko · 2022-05-13T20:32:38Z

packages/mc-scripts/src/commands/start.ts

+  });
+  const devServer = new WebpackDevServer(serverConfig, compiler);
+
+  await devServer.start();


We're using a promise now

emmenko · 2022-05-13T20:33:46Z

packages/mc-scripts/src/utils/graphql-requests.ts

+import type {
+  TCreateCustomApplicationFromCliMutation,
+  TCreateCustomApplicationFromCliMutationVariables,
+  TCustomApplicationDraftDataInput,
+  TFetchCustomApplicationFromCliQuery,
+  TFetchCustomApplicationFromCliQueryVariables,
+  TUpdateCustomApplicationFromCliMutation,
+  TUpdateCustomApplicationFromCliMutationVariables,
+} from '../generated/settings';
+import type {
+  TFetchMyOrganizationsFromCliQuery,
+  TFetchMyOrganizationsFromCliQueryVariables,
+} from '../generated/core';


Using the generated types.

CarlosCortizasCT · 2022-06-06T15:13:20Z

packages/mc-scripts/src/commands/config-sync.ts

@@ -110,6 +115,8 @@ const configSync = async () => {
      data,
    });

+    if (!createdCustomApplication) return;


Not sure about this line. I don't know when the createCustomApplication can return a null value and have not raised an error.
Even in that scenario, we would just exit the process and not tell the use what happened.

I guess we would need to know when this use case can happen and, if it really exists, add an explanatory message before exiting the process.

The graphql client throws an error, so we don't need to explicitly handle this case.
Not sure if this check is really necessary. Maybe TS complains about something? Could you double check?

So, that line was included in this commit due to some Typescript checking;

createdCustomApplication is the result of calling createCustomApplication function which returns an optional object.

This is the function implementation:

const createCustomApplication = async ({ mcApiUrl, token, organizationId, data, }: TCreateCustomApplicationOptions) => { const variables = { organizationId, data, }; try { const createdCustomAppData = await graphQLClient(mcApiUrl, token).request< TCreateCustomApplicationFromCliMutation, TCreateCustomApplicationFromCliMutationVariables >(CreateCustomApplicationFromCli, variables); return createdCustomAppData.createCustomApplication; } catch (error) { if (error instanceof Error) { throw new Error(error.message); } throw error; } };

And this is the definition for the TCreateCustomApplicationFromCliMutation type:

export type TCreateCustomApplicationFromCliMutation = { __typename?: 'Mutation'; createCustomApplication?: { __typename?: 'RestrictedCustomApplicationForOrganization'; id: string; } | null; };

So, the return statement createdCustomAppData.createCustomApplication can be technically null.

If we are sure that sending the create mutation will either work or raise an error, perhaps we can change the type definition to state createCustomApplication field is not optional.

This is the mutation sent:

mutation CreateCustomApplicationFromCli( $organizationId: String! $data: CustomApplicationDraftDataInput! ) { createCustomApplication(organizationId: $organizationId, data: $data) { id } }

@Rhotimee What's your take about updating the TCreateCustomApplicationFromCliMutation so the createCustomApplication is not optional?

These types are generated so we can't change them. I'll check again if we can make the return type non optional.

@Rhotimee What's your take about updating the TCreateCustomApplicationFromCliMutation so the createCustomApplication is not optional?

Sorry, I missed the tagged comment.

The line is not necessary, IMO. It was just added because of the type check error.
I don't think it matters what we return there because an error will be thrown anyway if the custom application is not created.

I pushed a commit with some small improvements: 4b66fa1

Making the type non optional is a bit tricky and I don't think it's worth the effort in this case. I left a comment with a note about the need for the check.

packages/mc-scripts/src/utils/user-agent.ts

CarlosCortizasCT

What a huge work in this PR 😮 💪

I just left a single comment to double check something I didn't fully understand.

kark

Wowzers! 🎉

packages/mc-scripts/src/commands/build.ts

packages/mc-scripts/src/cli.ts

packages/mc-scripts/src/commands/compile-html.ts

Rhotimee

Looks great 💯

…est util

…orts more decoupled

emmenko · 2022-06-21T12:29:36Z

Thanks @kark

emmenko commented May 13, 2022

View reviewed changes

packages/mc-scripts/src/commands/serve.ts

const port = 3001;

async function run() {

Copy link

Member Author

emmenko May 13, 2022

Choose a reason for hiding this comment

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

Nothing changed here

github-actions bot temporarily deployed to Preview May 13, 2022 20:31 Destroyed

emmenko commented May 13, 2022

View reviewed changes

github-actions bot temporarily deployed to Preview May 13, 2022 20:32 Destroyed

emmenko commented May 13, 2022

View reviewed changes

emmenko self-assigned this May 13, 2022

github-actions bot temporarily deployed to Preview May 13, 2022 21:08 Destroyed

github-actions bot temporarily deployed to Preview May 14, 2022 19:47 Destroyed

emmenko force-pushed the nm-ts-mc-scripts branch from 5f62e81 to a784174 Compare May 17, 2022 11:19

github-actions bot temporarily deployed to Preview May 17, 2022 11:27 Destroyed

github-actions bot temporarily deployed to Preview May 24, 2022 16:55 Destroyed

github-actions bot temporarily deployed to Preview May 24, 2022 17:09 Destroyed

github-actions bot temporarily deployed to Preview May 27, 2022 09:21 Destroyed

github-actions bot temporarily deployed to Preview May 27, 2022 12:57 Destroyed

emmenko marked this pull request as ready for review May 27, 2022 18:39

emmenko force-pushed the nm-ts-mc-scripts branch from 7dd2f5e to 81f5dd9 Compare June 3, 2022 09:57

emmenko requested a review from a team June 3, 2022 09:58

github-actions bot temporarily deployed to Preview June 3, 2022 10:02 Destroyed

CarlosCortizasCT reviewed Jun 6, 2022

View reviewed changes

CarlosCortizasCT reviewed Jun 7, 2022

View reviewed changes

packages/mc-scripts/src/utils/user-agent.ts Show resolved Hide resolved

CarlosCortizasCT reviewed Jun 7, 2022

View reviewed changes

kark approved these changes Jun 21, 2022

View reviewed changes

packages/mc-scripts/src/commands/build.ts Show resolved Hide resolved

packages/mc-scripts/src/cli.ts Show resolved Hide resolved

packages/mc-scripts/src/commands/compile-html.ts Outdated Show resolved Hide resolved

Rhotimee approved these changes Jun 21, 2022

View reviewed changes

emmenko and others added 19 commits June 21, 2022 14:13

refactor(mc-scripts): migrate to TS

c780cfa

refactor(mc-scripts): use cac library for managing cli workflow

d182234

refactor(mc-scripts): update generated types and migrate graphql requ…

b9ca82a

…est util

chore: clean up unused scripts

885deea

chore: more clean up

9194b32

refactor: define types for react-dev-utils

b8e9431

docs: changeset

4d3bac2

refactor(mc-scripts): more entry points to keep bundle chunks and imp…

faf1320

…orts more decoupled

refactor: use tsconfig base node v16 for node packages

2a80d78

refactor(mc-script): migrate utils

509e322

refactor(mc-script): migrate login command

0b40cfe

refactor(mc-script): migrate config:sync command to typescript

c648b17

chore(mc-script): run prettier

cb7198e

refactor(mc-scripts): type fixes and small improvements

33350bc

chore(mc-scripts): missing deps

2e65238

chore: formatting

4a38a11

refactor(mc-scripts): simplify error handling

07d0eb4

refactor(mc-scripts): adjust ts config and import

ccd906e

docs: changeset

438fc3e

emmenko force-pushed the nm-ts-mc-scripts branch from 4b66fa1 to 438fc3e Compare June 21, 2022 12:29

github-actions bot deployed to Preview June 21, 2022 12:34 View deployment

chore: bump timeout of link checker to 8min

d184a24

github-actions bot deployed to Preview June 21, 2022 13:54 View deployment

emmenko merged commit 20e648d into main Jun 21, 2022

emmenko deleted the nm-ts-mc-scripts branch June 21, 2022 15:30

ghost mentioned this pull request Jun 21, 2022

Version Packages #2643

Merged

emmenko mentioned this pull request Jul 4, 2022

fix(mc-scripts): parsing of CLI options casing #2680

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: migrate `mc-scripts` package to TypeScript #2613

refactor: migrate `mc-scripts` package to TypeScript #2613

emmenko commented May 13, 2022 •

edited

Loading

changeset-bot bot commented May 13, 2022 •

edited

Loading

emmenko May 13, 2022

emmenko May 13, 2022

emmenko May 13, 2022

emmenko May 13, 2022

emmenko May 13, 2022

emmenko May 13, 2022

emmenko May 13, 2022

emmenko May 13, 2022

github-actions bot commented May 13, 2022 •

edited

Loading

emmenko May 13, 2022

emmenko May 13, 2022

CarlosCortizasCT Jun 6, 2022

emmenko Jun 7, 2022

CarlosCortizasCT Jun 7, 2022

CarlosCortizasCT Jun 10, 2022

emmenko Jun 20, 2022

Rhotimee Jun 20, 2022

emmenko Jun 20, 2022

CarlosCortizasCT left a comment

kark left a comment

Rhotimee left a comment

emmenko commented Jun 21, 2022


		const appDirectory = fs.realpathSync(process.cwd());

		async function run(options: TCliCommandCompileHtmlOptions = {}) {

refactor: migrate mc-scripts package to TypeScript #2613

refactor: migrate mc-scripts package to TypeScript #2613

Conversation

emmenko commented May 13, 2022 • edited Loading

changeset-bot bot commented May 13, 2022 • edited Loading

🦋 Changeset detected

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CarlosCortizasCT left a comment

Choose a reason for hiding this comment

kark left a comment

Choose a reason for hiding this comment

Rhotimee left a comment

Choose a reason for hiding this comment

emmenko commented Jun 21, 2022

refactor: migrate `mc-scripts` package to TypeScript #2613

refactor: migrate `mc-scripts` package to TypeScript #2613

emmenko commented May 13, 2022 •

edited

Loading

changeset-bot bot commented May 13, 2022 •

edited

Loading

github-actions bot commented May 13, 2022 •

edited

Loading