[Themes] Remove Ruby invocation from the `app dev` command #4592

jamesmengo · 2024-10-07T17:26:12Z

WHY are these changes introduced?

Closes https://github.com/Shopify/develop-advanced-edits/issues/362

WHAT is this pull request doing?

Remove legacy flag from app dev command
Delete theme-app-extension.js (legacy implementation)
Update imports + tests

How to test your changes?

Try running app dev with the legacy flag. This should complain about a non-existent flag

Post-release steps

Measuring impact

How do we know this change was effective? Please choose one:

n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
Existing analytics will cater for this addition
PR includes analytics changes to measure impact

Checklist

I've considered possible cross-platform impacts (Mac, Linux, Windows)
I've considered possible documentation changes

github-actions · 2024-10-07T17:26:30Z

Thanks for your contribution!

Depending on what you are working on, you may want to request a review from a Shopify team:

Themes: @shopify/advanced-edits
UI extensions: @shopify/ui-extensions-cli
- Checkout UI extensions: @shopify/checkout-ui-extensions-api-stewardship
Hydrogen: @shopify/hydrogen
Other: @shopify/app-inner-loop

github-actions · 2024-10-07T22:41:48Z

Coverage report

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	72.8% (+0.29% 🔼)	8551/11746
🟡	Branches	69.75% (+0.66% 🔼)	4199/6020
🟡	Functions	71.81% (+0.04% 🔼)	2209/3076
🟡	Lines	73.13% (+0.32% 🔼)	8094/11068

Show new covered files 🐣

St.^❔	File	Statements	Branches	Functions	Lines
🟢	`...` / function-upload-url-generate.ts	100%	100%	100%	100%
🟢	`...` / active-app-release.ts	100%	100%	100%	100%
🟢	`...` / app-version-by-id.ts	100%	100%	100%	100%
🟢	`...` / app-versions.ts	100%	100%	100%	100%
🟢	`...` / apps.ts	100%	100%	100%	100%
🟢	`...` / create-app-version.ts	100%	100%	100%	100%
🟢	`...` / create-app.ts	100%	100%	100%	100%
🟢	`...` / create-asset-url.ts	100%	100%	100%	100%
🟢	`...` / organization.ts	100%	100%	100%	100%
🟢	`...` / release-version.ts	100%	100%	100%	100%
🟢	`...` / specifications.ts	100%	100%	100%	100%
🟢	`...` / user-info.ts	100%	100%	100%	100%
🟢	`...` / authorize.ts	100%	75%	100%	100%
🔴	`...` / post-auth.ts	44.12%	0%	0%	45.45%
🔴	`...` / redirect-listener.ts	14.75%	0%	33.33%	15%

Show files with reduced coverage 🔻

St.^❔	File	Statements	Branches	Functions	Lines
🟢	`...` / app.test-data.ts	91.4%	91.09% (-0.09% 🔻)	81.01%	90.8%
🟢	`...` / app.ts	87.07%	70.89% (-0.72% 🔻)	92%	88.37%
🟢	`...` / specification.ts	93.1% (-1.81% 🔻)	90.48%	87.5% (-0.5% 🔻)	92% (-2.12% 🔻)
🟢	`...` / function.ts	86.36% (-0.59% 🔻)	86.36%	83.33%	86.36% (-0.59% 🔻)
🟢	`...` / app-context.ts	100%	87.5% (-1.97% 🔻)	100%	100%
🟢	`...` / deploy.ts	87.18%	85% (-2.5% 🔻)	87.5%	89.19%
🟢	`...` / generate.ts	100%	80% (-2.14% 🔻)	100%	100%
🟢	`...` / show.ts	100%	66.67% (-8.33% 🔻)	100%	100%
🔴	`...` / extension.ts	55.26% (-0.29% 🔻)	50%	57.14%	56.76% (-0.06% 🔻)
🟢	`...` / theme-extension-args.ts	86.67%	50% (-12.5% 🔻)	100%	86.67%
🟡	`...` / update-extension.ts	64.86% (-5.14% 🔻)	54.55% (-3.79% 🔻)	60%	68.75% (-5.54% 🔻)
🟡	`...` / build.ts	74.49%	59.09% (-2.27% 🔻)	75.76%	72.22%
🟢	`...` / extension.ts	91.4% (+0.09% 🔼)	73.58%	91.3% (-0.36% 🔻)	91.21% (+0.1% 🔼)
🔴	`...` / app-management-client.ts	20.75% (-0.09% 🔻)	10.26% (+0.5% 🔼)	22.58% (-0.25% 🔻)	19% (-0.09% 🔻)
🔴	`...` / partners-client.ts	26.87% (-0.2% 🔻)	40%	18.18% (-0.34% 🔻)	26.56% (-0.21% 🔻)
🟢	`...` / session.ts	81.01% (-1.99% 🔻)	72.32% (-0.41% 🔻)	92.86%	80.67% (-2.09% 🔻)
🟢	`...` / ConcurrentOutput.tsx	98.39% (-1.61% 🔻)	90.91% (-4.55% 🔻)	100%	98.33% (-1.67% 🔻)
🟡	`...` / fs.ts	62.5%	84.62% (-7.05% 🔻)	58.97%	62.5%
🟢	`...` / admin.ts	82.76% (+1.51% 🔼)	37.5% (-2.5% 🔻)	90%	85.71% (+1.84% 🔼)

Test suite run success

1947 tests passing in 876 suites.

Report generated by 🧪jest coverage report action from e7d58bf

github-actions · 2024-10-08T00:02:48Z

We detected some changes at either packages/*/src or packages/cli-kit/assets/cli-ruby/** and there are no updates in the .changeset.
If the changes are user-facing, run "pnpm changeset add" to track your changes and include them in the next release CHANGELOG.

jamesmengo · 2024-10-08T00:06:37Z

packages/app/src/cli/services/dev/processes/setup-dev-processes.test.ts

@@ -26,12 +26,15 @@ import {describe, test, expect, beforeEach, vi} from 'vitest'
 import {ensureAuthenticatedAdmin, ensureAuthenticatedStorefront} from '@shopify/cli-kit/node/session'
 import {Config} from '@oclif/core'
 import {getEnvironmentVariables} from '@shopify/cli-kit/node/environment'
+import {isStorefrontPasswordProtected} from '@shopify/theme'


Updated these tests to assert that the process list that is built uses the TS implementation coming from theme-app-extension-next rather than the legacy implementation

jamesmengo · 2024-10-08T00:07:10Z

packages/app/src/cli/services/dev/processes/theme-app-extension-next.ts

@@ -22,7 +25,6 @@ interface HostThemeSetupOptions {
 storeFqdn: string
 theme?: string
 themeExtensionPort?: number
- developerPlatformClient: DeveloperPlatformClient


jamesmengo · 2024-10-08T00:08:48Z

packages/app/src/cli/services/dev/processes/theme-app-extension-next.ts

 },
 }
 }

+export const runThemeAppExtensionsServer: DevProcessFunction<ThemeAppExtensionServerOptions> = async (
+ _,


The first arg is an object with {stdout, stderr, abortSignal} - do we need to connect any of these?

ctrl+c or q both correctly terminate the process.

I believe our final goal is to update the development server to accept stdout and stderr, so for app dev, we can pass them — that will be addressed by this issue: https://github.com/Shopify/develop-advanced-edits/issues/366, correct?

Yep! Just needed to confirm with the foundations team but we're good to go :)

nickwesselman · 2024-10-15T15:58:59Z

/snapit

github-actions · 2024-10-15T16:01:32Z

🫰✨ Thanks @nickwesselman! Your snapshot has been published to npm.

Test the snapshot by intalling your package globally:

pnpm i -g @shopify/[email protected]

After installing, validate the version by running just shopify in your terminal
If the versions don't match, you might have multiple global instances installed.
Use which shopify to find out which one you are running and uninstall it.

nickwesselman · 2024-10-15T18:26:31Z

/snapit

github-actions · 2024-10-15T18:28:22Z

🫰✨ Thanks @nickwesselman! Your snapshot has been published to npm.

Test the snapshot by intalling your package globally:

pnpm i -g @shopify/[email protected]

After installing, validate the version by running just shopify in your terminal
If the versions don't match, you might have multiple global instances installed.
Use which shopify to find out which one you are running and uninstall it.

nickwesselman · 2024-10-15T18:36:02Z

/snapit

github-actions · 2024-10-15T18:37:52Z

🫰✨ Thanks @nickwesselman! Your snapshot has been published to npm.

Test the snapshot by intalling your package globally:

pnpm i -g @shopify/[email protected]

After installing, validate the version by running just shopify in your terminal
If the versions don't match, you might have multiple global instances installed.
Use which shopify to find out which one you are running and uninstall it.

karreiro

Thank you for this PR, @jamesmengo! LGTM and works as expected as well :)

karreiro · 2024-10-16T16:46:11Z

packages/app/src/cli/services/dev/processes/theme-app-extension-next.ts

 },
 }
 }

+export const runThemeAppExtensionsServer: DevProcessFunction<ThemeAppExtensionServerOptions> = async (
+ _,


I believe our final goal is to update the development server to accept stdout and stderr, so for app dev, we can pass them — that will be addressed by this issue: https://github.com/Shopify/develop-advanced-edits/issues/366, correct?

Rename theme-app-extension-next to theme-app-extension

jamesmengo added 2 commits October 7, 2024 10:16

remove legacy flag

87adda4

Update imports to remove legacy TAE setup

929b5f8

jamesmengo requested a review from karreiro October 7, 2024 17:26

jamesmengo self-assigned this Oct 7, 2024

jamesmengo added the #gsd:40767 Fortify local development experience for Liquid themes label Oct 7, 2024

jamesmengo marked this pull request as ready for review October 7, 2024 17:26

jamesmengo requested a review from frandiox October 7, 2024 17:26

jamesmengo force-pushed the jm/remove-ruby/appdev branch from 8e28ef9 to cb7a9e7 Compare October 7, 2024 17:36

jamesmengo added 3 commits October 7, 2024 17:02

Update docs

303cf62

Remove import from tests

c88388c

Remove legacy theme-app-extension file

bb5a431

jamesmengo force-pushed the jm/remove-ruby/appdev branch from dd42c42 to cbf552a Compare October 8, 2024 00:02

jamesmengo commented Oct 8, 2024

View reviewed changes

Update tests and follow DevProcess patterns in theme-app-extension-next

40749eb

jamesmengo force-pushed the jm/remove-ruby/appdev branch from cbf552a to 40749eb Compare October 8, 2024 00:10

Rename theme-app-extension-next to theme-app-extension

d7671f9

jamesmengo mentioned this pull request Oct 8, 2024

Rename theme-app-extension-next to theme-app-extension #4593

Merged

Fix AsyncLocalStorage with file watching

c40310f

nickwesselman requested a review from a team as a code owner October 15, 2024 18:25

fix listener abort

eeafbdb

karreiro approved these changes Oct 16, 2024

View reviewed changes

jamesmengo enabled auto-merge October 16, 2024 17:49

Merge pull request #4593 from Shopify/jm/remove-ruby/appdev-rename

e7d58bf

Rename theme-app-extension-next to theme-app-extension

jamesmengo requested a review from a team October 16, 2024 18:52

jamesmengo disabled auto-merge October 16, 2024 21:40

jamesmengo added this pull request to the merge queue Oct 16, 2024

Merged via the queue into main with commit f7ff1c1 Oct 16, 2024
36 checks passed

jamesmengo deleted the jm/remove-ruby/appdev branch October 16, 2024 21:47

shopify-shipit bot temporarily deployed to nightly October 17, 2024 09:31 Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Themes] Remove Ruby invocation from the `app dev` command #4592

[Themes] Remove Ruby invocation from the `app dev` command #4592

jamesmengo commented Oct 7, 2024 •

edited

Loading

github-actions bot commented Oct 7, 2024

github-actions bot commented Oct 7, 2024 •

edited

Loading

github-actions bot commented Oct 8, 2024

jamesmengo Oct 8, 2024

jamesmengo Oct 8, 2024

jamesmengo Oct 8, 2024

karreiro Oct 16, 2024

jamesmengo Oct 16, 2024

nickwesselman commented Oct 15, 2024

github-actions bot commented Oct 15, 2024

nickwesselman commented Oct 15, 2024

github-actions bot commented Oct 15, 2024

nickwesselman commented Oct 15, 2024

github-actions bot commented Oct 15, 2024

karreiro left a comment

karreiro Oct 16, 2024

[Themes] Remove Ruby invocation from the app dev command #4592

[Themes] Remove Ruby invocation from the app dev command #4592

Conversation

jamesmengo commented Oct 7, 2024 • edited Loading

WHY are these changes introduced?

WHAT is this pull request doing?

How to test your changes?

Post-release steps

Measuring impact

Checklist

github-actions bot commented Oct 7, 2024

github-actions bot commented Oct 7, 2024 • edited Loading

Coverage report

Test suite run success

github-actions bot commented Oct 8, 2024

jamesmengo Oct 8, 2024

Choose a reason for hiding this comment

jamesmengo Oct 8, 2024

Choose a reason for hiding this comment

jamesmengo Oct 8, 2024

Choose a reason for hiding this comment

karreiro Oct 16, 2024

Choose a reason for hiding this comment

jamesmengo Oct 16, 2024

Choose a reason for hiding this comment

nickwesselman commented Oct 15, 2024

github-actions bot commented Oct 15, 2024

nickwesselman commented Oct 15, 2024

github-actions bot commented Oct 15, 2024

nickwesselman commented Oct 15, 2024

github-actions bot commented Oct 15, 2024

karreiro left a comment

Choose a reason for hiding this comment

karreiro Oct 16, 2024

Choose a reason for hiding this comment

[Themes] Remove Ruby invocation from the `app dev` command #4592

[Themes] Remove Ruby invocation from the `app dev` command #4592

jamesmengo commented Oct 7, 2024 •

edited

Loading

github-actions bot commented Oct 7, 2024 •

edited

Loading