-
Notifications
You must be signed in to change notification settings - Fork 143
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
Theme check: command doesn't inspect any file after CLI upgrade #3497
Theme check: command doesn't inspect any file after CLI upgrade #3497
Comments
I think we somehow made the path argument required as a mistake. I believe there's a fix coming to the next release. How about Also, is there a Don't you need to |
How about shopify theme check --path .? Same thing or no? Also, is there a .git in that folder? Don't you need to dir into the theme before running the command? New info from my colleagues, it seems that the issue happens only on Windows. Theme check still works on their Mac computers. |
Ha that's interesting. Thanks for the info. Reopening. We should see what's up with windows. Transferring to CLI since this seems to be a CLI issue. Does the VS Code extension work fine in that repo? https://marketplace.visualstudio.com/items?itemName=Shopify.theme-check-vscode |
Yes, the extension works fine in that repo. I don't know if I can make it check all the files in the theme, instead of just the opened files. About a couple of weeks ago the vscode extension started showing new errors in our code, like HTML tags that were not closed correctly, or unused variables. This was great, but unfortunately also started to show way too many false errors:
Know that those false positives only appear on some of the repos. I don't know if there is something in the .theme-check.yml hiding the errors on other repos. I suppose there isn't. |
This usually happens because of not specifying the
That or you don't have a The other problem is the same thing. Because we can't infer the root, we can't find the translations and that breaks the translation check. |
Thanks! That fixed those checks made by the extension. The failing repos didn't even had a .theme-check.yml. Although the root of the theme was the root of the vs code project, so it would be great if the extension could run using It worked regardless of the existence of |
We were getting "Can't find file size" for assets/app.js errors being logged. That was because the function wasn't being called with an absolute path but with a relative path. For fuck's sake. I refactored our root finding algo for a long time thinking it was the cause. It wasn't. It was this fucking dumb implementation bug. Fixes Shopify/cli#3497
Tagged the wrong issue with PR |
Ok, I had the same issue and have figured out the problem. Basically, the issue is in the theme-check-node > src > index.ts - getTheme function. On windows, the glob function uses has an issue with the path separator i.e '' - glob fails to pick up any files from the folders at all. Have fixed it below - just have to replace the getTheme function. Will raise a PR shortly once I test it end to end on windows. export async function getTheme(config: Config): Promise<Theme> {
// On windows machines - the separator provided by path.join is '\'
// however the glob function fails silently since '\' is used to escape glob character
// as mentioned in the documentation of node-glob
// the path is normalised and '\' are replaced with '/' and then passed to the glob function
const normalized_path = path.normalize(path.join(config.root, '**/*.{liquid,json}')).replace(/\\/g, '/');
const paths = await asyncGlob(normalized_path).then((result) =>
// Global ignored paths should not be part of the theme
result.filter((filePath) => !isIgnored(filePath, config)),
);
const sourceCodes = await Promise.all(paths.map(toSourceCode));
return sourceCodes.filter((x): x is LiquidSourceCode | JSONSourceCode => x !== undefined);
} |
I think this issue should be reopened. I and a colleague are having the same problem. Notably, we both use Windows and a colleague who doesn't have this issue is on Mac. We can run |
I have Windows and theme check is working for me. Try removing your .theme-check.yml file and running |
This needs to be re-opened! I am experiencing the same issue on Windows 10 with the latest version of the CLI |
hi @charlespwd , can you reopen this issue? it is happening again. Confirmed for versions 3.71.5 and above on Windows. |
Describe the bug
Running the command line 'shopify theme check' is not inspecting files, returning a Success message , 0 files inspected with no offenses.
Source
It happens on all Theme projects, even Dawn. It also happens to my other colleagues on their computers.
Expected behaviour
The command should output a large list of errors and suggestions.
Actual behaviour
The command is not inspecting files.
The command returns:
Success
Theme Check Summary.
0 files inspected with no offenses found.
Debugging information
Additional context
The command was working fine until I upgraded shopify CLI from v3.48.x to v3.52.x. It is broken since then.
See an example with a fresh start of a Dawn theme:
The text was updated successfully, but these errors were encountered: