Add optional 2-way sync between the CLI (theme serve) and the Theme Editor
#2174
Conversation
- The `RemoteWatcher` to pull files in the given interval
- Notice the `RemoteWatcher` relies in the `ThreadPool` (we will check
it in a future commit
- The `Syncer` to avoid overriding JSON files
- Use the new `Checksums` class
- Use the new `IgnoreHelper` module
- Use the new `JsonDeleteHandler` module (we will check it in a future
commit)
- Use the new `JsonUpdateHandler` module (we will check it in a future
commit)
- Introduce a new queueable method (`union_merge`)
- Update the the `upload_theme!` to split the updates between JSON and
non-JSON files (JSON files are handled by the modules mentioned
above)
- Add new `merge_file` wrapper into `ShopifyCLI::Git`
- Use the new wrapper in the `Theme::Syncer::Merger` to merge theme
files
…Handler` modules to handle JSON deletes/updates in the `Syncer` by asking users how to proceed (when JSON files should not be overwritten)
…nUpdateHandler`:
- `SelectDeleteStrategy` asks about the strategy to handle deleted
files
- `SelectUpdateStrategy` asks about the strategy to handle updated
files
- `BaseStrategyForm` supports holds the logic shared by forms mentioned
above
- `ApplyToAll` asks users if they want to apply the selected value to
all other assets (to avoid asking users too much, in some context
they may have up to 50 updated files)
karreiro
requested review from
macournoyer,
a team,
pepicrft and
gonzaloriestra
and removed request for
a team,
pepicrft and
gonzaloriestra
There was a problem hiding this comment.
Damn! 👏 Nice work. Top hatted... It's magical 🪄
I skimmed through some parts of the code, but core logic looks right.
What do you think about dropping the value in --theme-editor-sync=SEC, just --theme-editor-sync w/ a default? I don't see why anyone would want to tweak that? And we can change the default later if it's polling too much.
|
Awesome idea 🔥 Thank you, @macournoyer! I've applied your suggestion in a new commit :) |
and one of the files is broken
…ime [2] Update '--theme-editor-sync' help message in the messages.rb [2]
|
@karreiro thanks so much for this, it's so helpful! |
|
This really should be more prominent in the documents. I thought I'd lost a couple of hours' work and then discovered this. Resigned to redoing everything, I noticed that there are previous versions of files in the code editor, and I was able to recover my data and schema files. If Shopify wants to be taken seriously, they really need to sort this sort of thing out. |
git log -12). They do not reflect the order of implementation but a more logical path to follow.WHY are these changes introduced?
Currently, when developers run
theme serve, the CLI replaces the remote development theme with the local version.Many developers expect that (1-way sync) behavior and already have tooling/workflows relying on that.
However, other developers prefer to rely on a 2-way sync logic. In other words: when something changes in the Theme Editor, the CLI (
theme serve) should auto get the remote updated version; and when something changes in the local files, the CLI should auto pushes the updated version.Thus, this PR adds an optional 2-way sync capabilities between the CLI (
theme serve) and the Theme Editor with the new--theme-editor-syncflag.WHAT is this pull request doing?
This PR approaches the 2-way sync with 3 pillars:
1. The
theme servestartupWhen developers run
theme servewith the--theme-editor-syncflag, the CLI checks (before pushing the local files to the development theme) if there are changes on remote.jsonfiles.If so, the CLI prompts users to check which version they prefer to keep, or even if they want a merged version of files (ref:
JsonUpdateHandlerandJsonDeleteHandler).2. The
theme servepulling mechanismWhen developers run
theme servewith the--theme-editor-syncflag, the CLI adopts will an approach similar to --pull-json-interval to pull.jsonfiles (ref:RemoteWatcher).3. the Theme Editor conflicts mechanism
The CLI already automatically uploads the updated
.jsonfile when partners locally change them, and the Theme Editor already has a mechanism to avoid conflicts. Thus, no changes will be performed on this side.Goals and non-goals
This PR introduces the
--theme-editor-syncflag to help developers (that expect 2-way sync features) on avoiding scenarios where they lose their data/work (e.g., when re-runtheme serveafter working in the Theme Editor).It focuses on performing checks/helpful-actions during the start-up and auto-pulling files when the theme serve is running.
This PR doesn't aim to introduce consensus ordering between the CLI and the Theme Editor, mutation history, and ensure conflict-free features in the
Syncer.How to test your changes?
Sync test case
shopify theme serve --theme-editor-syncStartup case
shopify theme serve --theme-editor-syncOther test cases:
Many updated files localy test case
shopify theme serve --theme-editor-syncFiles removed locally (restore test case)
shopify theme serve --theme-editor-syncFiles removed locally (delete test case)
shopify theme serve --theme-editor-synctemplates/index.jsonlocallytemplates/index.jsonin the Theme Editortheme servewithout the--theme-editor-syncflag test caseshopify theme servewith modified files locallyCustomize this theme in the Theme Editor, and use 'theme pull' to get the changesPost-release steps
Include the
--theme-editor-syncin thetheme servedocumentation.Update checklist