Update ui_extension to support Unified config #2349

isaacroldan · 2023-07-05T11:59:32Z

WHY are these changes introduced?

Adds support for the new UI Extensions schema with the targeting field.

WHAT is this pull request doing?

Update ui_extension schema to support both extension_instance and targeting.
Internally keep working with extension_instance
Update ui_extension template to follow unified config

How to test your changes?

From main generate a new ui_extension (it'll have the old format)
From this branch, and on the same app, generate another `ui_extension (should have the new format)
app info should load both extensions without problems.

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
I've made sure that any changes to dev or deploy have been reflected in the internal flowchart.

github-actions · 2023-07-05T11:59:58Z

Thanks for your contribution!

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

Themes: @shopify/theme-code-tools
UI extensions: @shopify/ui-extensions-cli
- Checkout UI extensions: @shopify/checkout-ui-extensions-api-stewardship
Hydrogen: @shopify/hydrogen
Other: @shopify/cli-foundations

packages/app/src/cli/models/extensions/specifications/ui_extension.ts

vividviolet

@lemonmade @davejcameron Should we support inheriting metafields and settings from the global?

packages/app/src/cli/models/extensions/specifications/ui_extension.ts

github-actions · 2023-07-05T16:50:15Z

Coverage report

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	73.75% (+0.03% 🔼)	5535/7505
🟡	Branches	70.41% (+0.02% 🔼)	2675/3799
🟡	Functions	72.58% (+0.04% 🔼)	1427/1966
🟡	Lines	75.44% (+0.02% 🔼)	5267/6982

Show files with reduced coverage 🔻

St.^❔	File	Statements	Branches	Functions	Lines
🟢	`...` / app.ts	80.56% (-0.53% 🔻)	53.33%	77.78% (-1.17% 🔻)	84.13% (-0.49% 🔻)
🟢	`...` / ui_extension.ts	97.83% (-2.17% 🔻)	81.82% (-18.18% 🔻)	100%	97.78% (-2.22% 🔻)
🟢	`...` / id-matching.ts	98.77%	75% (-5% 🔻)	100%	100%

Test suite run success

1297 tests passing in 637 suites.

Report generated by 🧪jest coverage report action from cadea7a

lemonmade · 2023-07-05T21:08:47Z

@vividviolet I think it would be nice to allow metafields at root that get inherited, as it would mean we can keep that part of a developer's existing extension the same across the transition. This is a feature we will eventually remove anyways, but if it's easy to each included extension, 👍

vividviolet · 2023-07-05T21:18:37Z

@lemonmade I just realize that there would be 3 levels of inheritance for Metafields for UI Extensions if we support setting it at the global, extension and extension.targetting level. Is that too confusing?

lemonmade · 2023-07-05T21:21:22Z

I would definitely be fine with just having it inherit from extension => extension.targeting, no other extension type does (or should) use this same metafields configuration mechanism.

vividviolet · 2023-07-05T21:37:56Z

Ok, I thought Function had metafields too but it turned out to be input.metafields: https://shopify.dev/docs/apps/functions/input-output/variables-queries#step-1-specify-the-metafield-to-use

@isaacroldan I think in summary these are all the fields that should be present, some of which need to be inherited either from the top-level or the extension-level.

api_version = "2023-04"
name = "Google maps validation"
description = "Validates location on Google maps"

[[extensions]]
handle = "google-maps-function"
type = "function"
# inherits top-level global api_version if not set
# inherits top-level global name if not set
# inherits top-level global description if not set
  [extensions.build]
  command = "cargo wasi build --release"
  path = "google-maps-function/dist/function.wasm"

  [[extensions.targeting]]
  export = "fetch"
  input_query = "google-maps-function/fetch.input.graphql"
  target = "purchase.validation.fetch"

  [[extensions.targeting]]
  export = "validate"                                         
  input_query = "google-maps-function/fetch.validate.graphql"  target = "purchase.validation.validate"

[[extensions]]
handle = "google-maps-ui"
type = "ui-extension"
# inherits top-level global api_version if not set
# inherits top-level global name if not set
# inherits top-level global description if not set

[[extensions.metafields]]
namespace = "my-namespace"
key = "my-key"

  [[extensions.targeting]]
  module = "google-maps-ui/settings.jsx"
  target = "purchase.validation.render-settings"
  # inherits extension-level metafields if not set

packages/app/src/cli/models/extensions/specifications/ui_extension.ts

isaacroldan · 2023-07-12T12:07:23Z

Updated! The ui_extension schema now supports either a targeting or a extension_points array.

If using targeting then it is transformed to extension_points internally. Extension level metafields are inherited in the target if not present.

All other top-level fields are flattened and included in the config of each extension in the array when parsing it. (name, description, api_version...)

I'll update the tests later today

github-actions · 2023-07-12T14:52:14Z

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.

vividviolet

I'm seeing the extension point metafields unexpectedly overriding the extension level one. I tested this config

name = "action-extension"
api_version = "unstable"
[[extensions]]
  type = "ui_extension"
  [[metafields]]
  namespace = "my-namespace"
  key = "my-key"
  [[metafields]]
  namespace = "my-namespace"
  key = "my-other-key"
  [[extensions.targeting]]
  target = "admin.product.item.block"
  module = "./index.jsx"
  [[extensions.targeting]]
  target = "admin.product.index.action"
  module = "./index.jsx"
  [[extensions.metafields]]
  namespace = "my-specific-namespace"
  key = "my-specific-key"

I'm seeing this payload in the Dev Server

{
        "extensions": [
            {
                "extensionPoints": [
                    {
                        "target": "admin.product.item.block",
                        "module": "./index.jsx",
                        "metafields": [
                            {
                                "namespace": "my-specific-namespace",
                                "key": "my-specific-key"
                            }
                        ],
                    },
                    {
                        "target": "admin.product.index.action",
                        "module": "./index.jsx",
                        "metafields": [
                            {
                                "namespace": "my-specific-namespace",
                                "key": "my-specific-key"
                            }
                        ],
                    }
                ],
                "metafields": [
                    {
                        "namespace": "my-specific-namespace",
                        "key": "my-specific-key"
                    }
                ],
                "type": "ui_extension",
                "externalType": "ui_extension",
                "uuid": "dev-4159f8c6-a95b-48e4-9102-d76d1933a392",
                "surface": "all",
                "title": "action-extension",
                "apiVersion": "unstable",
                "approvalScopes": []
            },
        ]
    },
}

This is what I would expect:

{
        "extensions": [
            {
                "extensionPoints": [
                    {
                        "target": "admin.product.item.block",
                        "module": "./index.jsx",
                        "metafields": [
                            {
                                "namespace": "my-namespace",
                                "key": "my-key"
                            },
                           {
                                "namespace": "my-specific-namespace",
                                "key": "my-other-key"
                            }
                        ],
                    },
                    {
                        "target": "admin.product.index.action",
                        "module": "./index.jsx",
                        "metafields": [
                            {
                                "namespace": "my-specific-namespace",
                                "key": "my-specific-key"
                            }
                        ],
                    }
                ],
                "metafields": [
                    {
                        "namespace": "my-namespace",
                        "key": "my-key"
                    },
                    {
                        "namespace": "my-namespace",
                        "key": "my-other-key"
                    }
                ],
                "type": "ui_extension",
                "externalType": "ui_extension",
                "uuid": "dev-4159f8c6-a95b-48e4-9102-d76d1933a392",
                "surface": "all",
                "title": "action-extension",
                "apiVersion": "unstable",
                "approvalScopes": []
            },
        ]
    },
}

vividviolet · 2023-07-12T16:09:04Z

packages/app/src/cli/models/extensions/specifications/ui_extension.test.ts

+        api_version: '2023-01' as const,
+        name: 'UI Extension',
+        type: 'ui_extension',
+        metafields: [{namespace: 'test', key: 'test'}],


Can we make sure that this stripped out of the payload when deploying/dev? We don't actually support this config at the extension level in the backend or front end. It's only supported in the TOML for convenience

It's not included in the deployConfig for sure, not sure about how we build the dev payload, but I didn't change that 🤔

Ok, I will take care of that on the PR I'm working on to update the dev payload to get rid of label on extension points

vividviolet · 2023-07-12T16:12:37Z

packages/app/templates/ui-extensions/projects/ui_extension/shopify.ui.extension.toml.liquid

 api_version = "2022-10"

 [capabilities]
 api_access = true
 network_access = true
 block_progress = true

-[[extension_points]]
-target = "Checkout::Dynamic::Render"
+[[extensions]]


I think we should just delete this template. We should be creating a new version of the Checkout extension in the template repo and expose it through Partner's instead of having it be tied to the CLI.

Is it safe If I remove it in this PR then?

I think so? No one should be using this publicly for Checkout but we better ask @jamesvidler and @kumar303. They should be working on a new template soon for Checkout.

Bundles is using it but @kmdavis is working on a new template. We just need to merge the new template before the next release of the CLI goes out.

No one is using this template publicly, yet. Though we have used to test things. Where would the new template be stored?

You'd put it here and then update Partner's to make it available: https://github.com/Shopify/partners/blob/master/app/operations/apps/templates/specifications/v304600.rb

isaacroldan · 2023-07-13T10:13:50Z

I think the config you posted is wrong @vividviolet, you are defining top-level metafields and extension-level metafields, but no target-level metafields.

What happens is that extension-level override top-level, we can discuss if this is correct or they should be merged.
But target-level never overrides anything else:

name = "action-extension"
api_version = "unstable"
[[extensions]]
  type = "ui_extension"
  [[metafields]] #  <-- this is top-level
  namespace = "top-level"
  key = "my-key"
  [[extensions.metafields]] # <-- extension-level
  namespace = "extension-level"
  key = "my-other-key"
  [[extensions.targeting]]
  target = "admin.product.item.block"
  module = "./index.jsx"
  [[extensions.targeting]]
  target = "admin.product.index.action"
  module = "./index.jsx"
  [[extensions.targeting.metafields]] # <-- target-level
  namespace = "target-level"
  key = "my-specific-key"

target-level will inherit extension-level if not present
extension-level will inherit from top-level if not present
top-level is ignored if extension-level is present
target-level won't inherit from top-level in any case

Does this make sense? let's discuss offline if not :)

vividviolet · 2023-07-13T16:14:16Z

@isaacroldan you're correct, I got the TOML wrong. Using this works as expected

name = "action-extension"
api_version = "internal"
[[extensions]]
  type = "ui_extension"
  [[extensions.metafields]]
  namespace = "my-namespace"
  key = "my-key"
  [[extensions.metafields]]
  namespace = "my-namespace"
  key = "my-other-key"

  [extensions.capabilities]
  network_access = true
  block_progress = true
  api_access = true

  [extensions.settings]
    [[settings.fields]]
    key = "field_key"
    type = "boolean"
    name = "field-name"
    [[settings.fields]]
    key = "field_key_2"
    type = "number_integer"
    name = "field-name-2"

  [[extensions.targeting]]
  target = "admin.product.item.block"
  module = "./index.jsx"

  [[extensions.targeting]]
  target = "admin.product.index.action"
  module = "./index.jsx"
  [[extensions.targeting.metafields]]
  namespace = "my-specific-namespace"
  key = "my-specific-key"

I think the only thing left to do is to drop support for setting the metafields at the top-level as this is a ui-extension concept only

vividviolet

Tophat'd both dev and deploy - everything works as expected

jamesvidler

I tophatted this by generating a new extension with an app I already had in spin (and an existing extension) and it generated the new extension with the new config just fine.

I was also able to run pnpm shopify app dev and see the extension render. 👍

What is strange though is that the other extension I have appears to be excluded now and ignored. Any idea what might be going on there? It's entirely possible the app structure is out of date, as I did not generate a new app.

This is the branch of the app I was using to test.

isaacroldan · 2023-07-14T09:12:26Z

@jamesvidler Looks like your old extension is missing the type = ui_extension. At the beginning we added a default value for the type, but have since removed it so we expect all extensions to have a type.
@vividviolet is this OK? we really don't want to have a default value for this.

jamesvidler · 2023-07-14T13:32:36Z

@jamesvidler Looks like your old extension is missing the type = ui_extension. At the beginning we added a default value for the type, but have since removed it so we expect all extensions to have a type. @vividviolet is this OK? we really don't want to have a default value for this.

Confirmed this was the issue I ran into. Adding the type in the "old config" made it work again.

vividviolet · 2023-07-14T14:51:18Z

@isaacroldan @jamesvidler I'm good with remove the fallback for setting the type in legacy ui_extension. One thing to note is that old versions of the ui_extension template has already been shipped with previous versions of the CLI. This means that when we turn the org beta UI Extension to 100% at Summer Editions users with old CLI versions will still see the old template. That's completely fine as it will continue to work with old CLI versions. They just need to bump their api_version before deploying as the new minimum supported version is 2023-04 for Checkout.

New ui extension union schema

ee7fb52

isaacroldan added 2 commits July 5, 2023 14:01

Extract settings schema

137c32b

Improve transform

f5fa23f

vividviolet reviewed Jul 5, 2023

View reviewed changes

isaacroldan added 2 commits July 5, 2023 18:05

Remove handle

4731b47

Update schema to override global configs

c8871a8

vividviolet reviewed Jul 5, 2023

View reviewed changes

packages/app/src/cli/models/extensions/specifications/ui_extension.ts Outdated Show resolved Hide resolved

packages/app/src/cli/models/extensions/specifications/ui_extension.ts Outdated Show resolved Hide resolved

Update schema

884bc52

vividviolet reviewed Jul 6, 2023

View reviewed changes

packages/app/src/cli/models/extensions/specifications/ui_extension.ts Outdated Show resolved Hide resolved

isaacroldan added 8 commits July 11, 2023 12:25

Merge branch 'main' into new-ui-extension-schema

9671be2

Merge branch 'main' into new-ui-extension-schema

25548b6

Simplify types in ui_extension

f236c79

Remove targeting from BaseSchema

c9ac3ec

Simplified ui_extension schema

51e46e1

Updated ui_extension template

e7d59af

Merge branch 'main' into new-ui-extension-schema

3b86b0f

Fix handle in template

f62639d

isaacroldan added 4 commits July 12, 2023 16:30

Clean up

a82f4b2

Add test for targeting schema

3cf2679

Add new test to validate targeting presence

6f05a27

Rever unnecessary changes

032c62b

isaacroldan marked this pull request as ready for review July 12, 2023 14:52

vividviolet reviewed Jul 12, 2023

View reviewed changes

isaacroldan added 4 commits July 13, 2023 20:09

Merge branch 'main' into new-ui-extension-schema

64e52d8

Make handle mandatory for ui_extensions

5dd591b

Unified schema only parses specific fields

c862b4b

Simplify unifiedSchema

06f4510

vividviolet approved these changes Jul 13, 2023

View reviewed changes

jamesvidler reviewed Jul 13, 2023

View reviewed changes

Make handle optional for legacy ui_extensions

ccc028e

isaacroldan added 2 commits July 14, 2023 11:29

Use BaseSchema for ui_extensions

5f1acc9

Merge branch 'main' into new-ui-extension-schema

85358ca

jamesvidler approved these changes Jul 14, 2023

View reviewed changes

isaacroldan added 4 commits July 14, 2023 17:30

Merge branch 'main' into new-ui-extension-schema

abe8994

Show error if type is missing

8eaf31d

Fix id-matching tests

917a941

Update tests with the new localIdentifier

cadea7a

isaacroldan added this pull request to the merge queue Jul 17, 2023

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 17, 2023

isaacroldan added this pull request to the merge queue Jul 17, 2023

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 17, 2023

isaacroldan added this pull request to the merge queue Jul 17, 2023

Merged via the queue into main with commit a659bfe Jul 17, 2023

isaacroldan deleted the new-ui-extension-schema branch July 17, 2023 11:53

shopify-shipit bot temporarily deployed to nightly July 17, 2023 15:17 Inactive

shopify-shipit bot temporarily deployed to production July 26, 2023 11:18 Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update ui_extension to support Unified config #2349

Update ui_extension to support Unified config #2349

isaacroldan commented Jul 5, 2023 •

edited

Loading

github-actions bot commented Jul 5, 2023

vividviolet left a comment

github-actions bot commented Jul 5, 2023 •

edited

Loading

lemonmade commented Jul 5, 2023

vividviolet commented Jul 5, 2023

lemonmade commented Jul 5, 2023

vividviolet commented Jul 5, 2023 •

edited

Loading

isaacroldan commented Jul 12, 2023

github-actions bot commented Jul 12, 2023

vividviolet left a comment

vividviolet Jul 12, 2023

isaacroldan Jul 13, 2023

vividviolet Jul 13, 2023

vividviolet Jul 12, 2023

isaacroldan Jul 13, 2023

vividviolet Jul 13, 2023

jamesvidler Jul 13, 2023

vividviolet Jul 13, 2023

isaacroldan commented Jul 13, 2023

vividviolet commented Jul 13, 2023 •

edited

Loading

vividviolet left a comment

jamesvidler left a comment

isaacroldan commented Jul 14, 2023

jamesvidler commented Jul 14, 2023

vividviolet commented Jul 14, 2023

Update ui_extension to support Unified config #2349

Update ui_extension to support Unified config #2349

Conversation

isaacroldan commented Jul 5, 2023 • 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 Jul 5, 2023

vividviolet left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 5, 2023 • edited Loading

Coverage report

Test suite run success

lemonmade commented Jul 5, 2023

vividviolet commented Jul 5, 2023

lemonmade commented Jul 5, 2023

vividviolet commented Jul 5, 2023 • edited Loading

isaacroldan commented Jul 12, 2023

github-actions bot commented Jul 12, 2023

vividviolet left a 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

Choose a reason for hiding this comment

isaacroldan commented Jul 13, 2023

vividviolet commented Jul 13, 2023 • edited Loading

vividviolet left a comment

Choose a reason for hiding this comment

jamesvidler left a comment

Choose a reason for hiding this comment

isaacroldan commented Jul 14, 2023

jamesvidler commented Jul 14, 2023

vividviolet commented Jul 14, 2023

isaacroldan commented Jul 5, 2023 •

edited

Loading

github-actions bot commented Jul 5, 2023 •

edited

Loading

vividviolet commented Jul 5, 2023 •

edited

Loading

vividviolet commented Jul 13, 2023 •

edited

Loading