Skip to content
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.

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

Modify checkout_ui template to use the new ui_extension type #1275

Merged
merged 1 commit into from
Feb 3, 2023

Conversation

vividviolet
Copy link
Member

@vividviolet vividviolet commented Feb 2, 2023

WHY are these changes introduced?

Fixes https://github.com/Shopify/ui-extensions-private/issues/2019

WHAT is this pull request doing?

  • Filter out ui_extension on the generate extension prompt and remove the template for ui_extension as it's currently hardcoded to Checkout. In the future we will have multiple templates for ui_extension but it's out of scope for this PR
  • Update checkout_ui_extension template to use the new ui_extension config and library
  • Update fixtures/app/extensions/checkout-ui to reflect the new template

How to test your changes?

  1. Run pnpm shopify app generate extension --path fixtures/app
  2. From the prompt, select Checkout UI
  3. Verify that the extension is generated successfully
  4. Run pnpm shopify app dev --path fixtures/app and open the Preview Url
  5. Verify that both checkout-ui extension and your new extension are running as ui_extensions - they each should have a preview link for the extension point

image

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
Copy link
Contributor

github-actions bot commented Feb 2, 2023

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-developer-tools
  • UI extensions: @shopify/ui-extensions-cli
    • Checkout UI extensions: @shopify/checkout-ui-extensions-api-stewardship
  • Hydrogen: @shopify/hydrogen
  • Other: @shopify/cli-foundations

@@ -59,7 +60,9 @@ async function generate(options: GenerateOptions) {
)
}
} else {
specifications = specifications.filter((spec) => app.extensionsForType(spec).length < spec.registrationLimit)
specifications = specifications
Copy link
Member Author

Choose a reason for hiding this comment

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

@isaacroldan we have to manually filter this out for the generate command for now since we don't yet have a way to enable multiple templates for a single specification

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense!
Did you see my proposal for a new template interface in the CLI? https://github.com/Shopify/internal-cli-foundations/discussions/518
If we agree on this, we might be able to work on it soon

@github-actions
Copy link
Contributor

github-actions bot commented Feb 2, 2023

Benchmark report

The following table contains a summary of the startup time for all commands.

Status Command Baseline (avg) Current (avg) Diff
🟢 app build 1266 ms 1280.67 ms 1.16 %
🟢 app deploy 1538 ms 1571.33 ms 2.17 %
🟢 app dev 1548 ms 1625.67 ms 5.02 %
🟢 app env pull 1414.67 ms 1389 ms -1.81 %
🟢 app env show 1383.67 ms 1453.33 ms 5.03 %
🟢 app generate extension 1526.67 ms 1499.67 ms -1.77 %
🟢 app generate schema 1490 ms 1494.67 ms 0.31 %
🟢 app info 1412.67 ms 1395.67 ms -1.2 %
🟢 app scaffold extension 1511.33 ms 1507.33 ms -0.26 %
🟢 app update-url 1374.33 ms 1400.33 ms 1.89 %
🟢 theme check 1161.33 ms 1163.33 ms 0.17 %
🟢 theme delete 1355.33 ms 1350.67 ms -0.34 %
🟢 theme dev 1340 ms 1348 ms 0.6 %
🟢 theme help-old 1157.67 ms 1178 ms 1.76 %
🟢 theme info 1227.67 ms 1227 ms -0.05 %
🟢 theme init 1172 ms 1220.33 ms 4.12 %
🟢 theme language-server 1152.33 ms 1166 ms 1.19 %
🟢 theme list 1329.33 ms 1313 ms -1.23 %
🟢 theme open 1353.67 ms 1330.67 ms -1.7 %
🟢 theme package 1215.33 ms 1285.67 ms 5.79 %
🟢 theme publish 1313 ms 1370.33 ms 4.37 %
🟢 theme pull 1320.67 ms 1333 ms 0.93 %
🟢 theme push 1303 ms 1320 ms 1.3 %
🟢 theme serve 1336.33 ms 1309 ms -2.05 %
🟢 theme share 1300 ms 1307.33 ms 0.56 %
🟢 webhook trigger 1335.67 ms 1325.33 ms -0.77 %

@vividviolet vividviolet force-pushed the update-checkout-ui-template branch 2 times, most recently from 549533f to b7a673b Compare February 3, 2023 16:45
@vividviolet vividviolet changed the title [WIP] Modify checkout_ui template to use the new ui_extension type Modify checkout_ui template to use the new ui_extension type Feb 3, 2023
@vividviolet vividviolet marked this pull request as ready for review February 3, 2023 16:49
@github-actions
Copy link
Contributor

github-actions bot commented Feb 3, 2023

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
71.83% (+0.03% 🔼)
3843/5350
🟡 Branches
69.42% (+0.04% 🔼)
1739/2505
🟡 Functions
70.17% (+0.04% 🔼)
1002/1428
🟡 Lines
73.07% (+0.04% 🔼)
3665/5016

Test suite run success

971 tests passing in 500 suites.

Report generated by 🧪jest coverage report action from af42f22

Remove `module` from the payload for deploying extension points as it's unused

Add changeset for release

Co-authored-by: Richard Powell <richardo@gmail.com>

Exclude ui_extension from generate extensions tests
@vividviolet vividviolet merged commit 3ebc870 into main Feb 3, 2023
@vividviolet vividviolet deleted the update-checkout-ui-template branch February 3, 2023 20:47
@shopify-shipit shopify-shipit bot temporarily deployed to nightly February 4, 2023 01:25 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to production February 6, 2023 15:28 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants