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

[Bug]: --theme-editor-sync running infinite loop #4107

Closed
2 tasks done
bigskillet opened this issue Jun 20, 2024 · 22 comments · Fixed by #4142 or #4160
Closed
2 tasks done

[Bug]: --theme-editor-sync running infinite loop #4107

bigskillet opened this issue Jun 20, 2024 · 22 comments · Fixed by #4142 or #4160
Assignees
Labels
important second highest severity for theme related bug Type: Bug Something isn't working

Comments

@bigskillet
Copy link

bigskillet commented Jun 20, 2024

Please confirm that you have:

  • Searched existing issues to see if your issue is a duplicate. (If you’ve found a duplicate issue, feel free to add additional information in a comment on it.)
  • Reproduced the issue in the latest CLI version.

In which of these areas are you experiencing a problem?

Theme

Expected behavior

Running shopify theme dev --store --theme-editor-sync should sync once on start.

Actual behavior

Sync gets stuck in an infinite loop:
Screenshot 2024-06-20 at 11 01 23 AM

Verbose output

Sorry, but --verbose runs in an infinite loop when combined with --theme-editor-sync. I'm not exactly sure what to paste here.

Reproduction steps

  1. shopify theme dev --store --theme-editor-sync

Operating System

macOS Sonoma 14.5

Shopify CLI version

3.62.0

@bigskillet bigskillet added the Type: Bug Something isn't working label Jun 20, 2024
@dreamer-cloud
Copy link

+1, same issue after updating to 3.62.0

@agentfitz
Copy link

agentfitz commented Jun 20, 2024

+1, same. Seeing infinite loops on settings_schema.json

Oddly, adding a trailing comma to the last item in our settings array in settings_schema.json seems to fix the issue 🤔 (found this clue on another thread)

@shopify/cli/3.59.3 darwin-arm64 node-v22.0.0

image

@stijns96
Copy link

Hi there 👋🏻

It probably has to due with the new trailing comma support. There are 2 solutions at the moment:

This is always happening with the settings_schema.json . If you have this with another file, you most likely have an illegal unicode in your code.

@mgmanzella mgmanzella added the important second highest severity for theme related bug label Jun 21, 2024
@bdahlinger
Copy link

this got my Vite assets/manifest.json file too, same endless loop, trailing comma fixed :/

@JamiesonRoberts
Copy link

As a note, something really does need to be done about this, because most code editors these days do auto formatting of files, which does not support the trailing comma as its not part of the spec of JSON. This is causing a massive headache of having to manually copy files when making changes, rather than letting the CLI handle those changes

@karreiro
Copy link
Contributor

karreiro commented Jul 3, 2024

👋 Hey everyone,

Thank you for your input and all the details about this issue. We've released a new version of the Shopify CLI some minutes ago with the fix (3.63.2). Please, if this problem persists, don't hesitate to reach out and share your --verbose output here.

Thanks again for reporting!

@lesterdefreitas
Copy link

Thanks @karreiro 👏🏻 You the man!

@lesterdefreitas
Copy link

image

Oh no... @karreiro

@karreiro
Copy link
Contributor

karreiro commented Jul 4, 2024

Thank you for sharing that, @lesterdefreitas! I've also noticed a similar scenario here.

My original PR, unfortunately, missed a detail related to assets/* files. The fix for that is applied here, so we may expect that to be resolved in a new CLI release soon.

@lesterdefreitas
Copy link

Ah, awesome, thanks dude!

@DanielDanielsson
Copy link

Oh sweet baby jesus.. Thank you guys!

@rcasimmons
Copy link

@karreiro this seems to have cropped back up with the latest release of CLI - both in the new and with --legacy flags.

Has the team not incorporated this fix?

I've attached my verbose log from the legacy cli:
Image

@dspeake
Copy link

dspeake commented Sep 27, 2024

Same here. We have some stores that are doing this across 100+ template files.
I personally don't think it was ever fully fixed, I still had issues with some .context.json files.

It feels to me as though it's to do with the new JSON comments that are added to the top of the file? Perhaps one side is trying to add them and the other side is trying to remove them? Just a thought.

@rcasimmons
Copy link

Same here. We have some stores that are doing this across 100+ template files. I personally don't think it was ever fully fixed, I still had issues with some .context.json files.

It feels to me as though it's to do with the new JSON comments that are added to the top of the file? Perhaps one side is trying to add them and the other side is trying to remove them? Just a thought.

Yeah that could be a plausible solution.

The weird thing is that before 3.67.X everything was working as it should but you're right - I don't think it was fully addressed.

Hopefully @karreiro can shed some light :)

@stijns96
Copy link

I feel like the CLI is getting worse and worse, unfortunately. Almost every release has a bug that breaks the entire CLI. If not at startup, then a minute later. Hot reload is sometimes executed multiple times and debugging is a hopeless task...

I'm glad we finally don't have to use brew anymore, but the quality of the CLI really needs to go up. Also, developing for multistore is not yet fully optimised unfortunately

@rcasimmons
Copy link

I feel like the CLI is getting worse and worse, unfortunately. Almost every release has a bug that breaks the entire CLI. If not at startup, then a minute later. Hot reload is sometimes executed multiple times and debugging is a hopeless task...

I'm glad we finally don't have to use brew anymore, but the quality of the CLI really needs to go up. Also, developing for multistore is not yet fully optimised unfortunately

Absolutely agree - the quality has really gone downhill - especially considering this is our only tool.

@ENG1N3X
Copy link

ENG1N3X commented Sep 27, 2024

I had the same issue with version 3.67.2

Adding a comma to all JSON files solved my problem

@agentfitz
Copy link

Yeah... what the heck? I just came back to my own codebase to make a small change and it is instantly going crazy, endlessly looping over json files. What gives?

Video evidence:
https://www.loom.com/share/71077c75b5d74c829a1bc3badda39fe7

@MaxDesignFR
Copy link

MaxDesignFR commented Sep 30, 2024

I too have this issue again. I got hundreds of json files going crazy. Impossible to use the CLI like that. It's been months and still no permanent fix was found, lame!

Here is a node script I placed in a .app folder in my project to add the trailing commas at the end of every json files. Could be better but does the job, and why in the first place do we have to go through this again? Make sure you use git to check the local changes.

const fs = require('fs');
const path = require('path');

// Directories to process
const directories = [
  path.join(__dirname, '../templates'),
  path.join(__dirname, '../locales'),
  path.join(__dirname, '../sections'),
];

function processJsonFile(filePath) {
  fs.readFile(filePath, 'utf8', (err, data) => {
    if (err) {
      console.error(`Error reading file ${filePath}:`, err);
      return;
    }

    // Remove any trailing commas before the final closing brace
    const cleanedData = data.replace(/,\s*([}\]])/g, '$1'); // Removes trailing commas before } or ]

    // Add a comma before the last closing curly brace, if needed
    const modifiedData = cleanedData.replace(/}\s*$/, ',\n}');

    // Write the modified content back to the file
    fs.writeFile(filePath, modifiedData, 'utf8', err => {
      if (err) {
        console.error(`Error writing file ${filePath}:`, err);
      } else {
        console.log(`Updated ${filePath}`);
      }
    });
  });
}

function processDirectory(dir) {
  fs.readdir(dir, { withFileTypes: true }, (err, files) => {
    if (err) {
      console.error(`Error reading directory ${dir}:`, err);
      return;
    }

    files.forEach(file => {
      const filePath = path.join(dir, file.name);
      if (file.isDirectory()) {
        processDirectory(filePath); // Recursively process subdirectories
      } else if (file.isFile() && path.extname(file.name) === '.json') {
        processJsonFile(filePath); // Process JSON file
      }
    });
  });
}

// Process directories
directories.forEach(processDirectory);

@rcasimmons
Copy link

I can't get my --verbose log at the moment @karreiro but this still appears to be happening with the --legacy flag - I have to use due to an issue I've logged here:
#4548

@karreiro
Copy link
Contributor

karreiro commented Oct 1, 2024

👋 Hey everyone,

It seems like a platform change has impacted the original fix applied to this issue.

If you're still facing the infinite loop in the most recent version of Shopify CLI, could you please share your --verbose output at #4537? With that, the team may get extra pointers about the issue you're facing. This issue should be addressed in Shopify CLI 3.67.3, but we want to make sure that everyone is no longer facing it.

I can't get my --verbose log at the moment @karreiro but this still appears to be happening with the --legacy flag

@rcasimmons, the --legacy flag was introduced as a convenient alternative to access the previous implementation of Shopify CLI commands (even with the most updated version of the CLI installed). However, the previous implementation of commands doesn't include recent fixes or features. So, removing the --legacy flag should resolve this issue. If it doesn't, please feel free to report that on #4537.

Thank you, everyone, for the quick feedback.

@rcasimmons
Copy link

👋 Hey everyone,

It seems like a platform change has impacted the original fix applied to this issue.

If you're still facing the infinite loop in the most recent version of Shopify CLI, could you please share your --verbose output at #4537? With that, the team may get extra pointers about the issue you're facing. This issue should be addressed in Shopify CLI 3.67.3, but we want to make sure that everyone is no longer facing it.

I can't get my --verbose log at the moment @karreiro but this still appears to be happening with the --legacy flag

@rcasimmons, the --legacy flag was introduced as a convenient alternative to access the previous implementation of Shopify CLI commands (even with the most updated version of the CLI installed). However, the previous implementation of commands doesn't include recent fixes or features. So, removing the --legacy flag should resolve this issue. If it doesn't, please feel free to report that on #4537.

Thank you, everyone, for the quick feedback.

Thanks @karreiro but as I've outlined I can't use the newer cli version due to the issue I linked - is there a chance the fix can be ported to the legacy cli version because I'm sure there will be people using --legacy who would require this fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
important second highest severity for theme related bug Type: Bug Something isn't working
Projects
None yet