Block directories in theme assets

[amcaplan]

WHY are these changes introduced?

This is the followup to #1881. We were failing in an unexpected way because we assumed users would not have subdirectories in their asset directories, rather than enforcing this behavior. @macournoyer graciously confirmed that asset subdirectories are not supported. So we're going to fail with a proper validation message.

WHAT is this pull request doing?

Before: Bugsnag

After:

Note the small additional benefit that following this PR, we're validating earlier in the process, preventing the frustration of syncing to 100% and only then discovering things are broken.

How to test your changes?

Create a theme with a subdirectory within assets/ and try to shopify theme serve

Update checklist

• I've added a CHANGELOG entry for this PR (if the change is public-facing)
• I've considered possible cross-platform impacts (Mac, Linux, Windows).
• I've left the version number as is (we'll handle incrementing this when releasing).

[amcaplan]

@MeredithCastile feel free to rewrite. The point is that the user has created a subdirectory in a location where our supported directory structure doesn't allow it (and apparently this is a hard requirement on our backend).

[MeredithCastile]

@amcaplan What's the task the user is trying to do when they'd hit this message? Adding assets to the subdirectory, right?

First draft pending your answer: "Assets can't be added to subdirectories. Add them directly to your assets folder (/assets)."

[amcaplan]

@MeredithCastile have a look at the PR message images, it happens when attempting to serve the theme. So they wouldn't necessarily have just added this file.

There's also the general point that while we see this in assets mostly (presumably users want to have more organization around their assets), technically we could see failures in other areas as well, if the user has violated the officially supported directory structure.

[MeredithCastile]

@amcaplan Ah, I understand now. Thanks for talking me through this earlier. Let's do this:

Your directory structure isn't supported.

Move any files to a parent folder, then delete unsupported subdirectories.

Required directory structure: https://shopify.dev/themes/architecture#directory-structure-and-component-types

Nice to have would be the indentation and bullet before the doc / link (as in the Figma screenshot).

[amcaplan]

Slight adjustment so they know what to delete:

[macournoyer]

That's awesome! So much better error message now. Thanks Ariel 🙏

[macournoyer]

EDIT: whoops, github hiccup... sorry for the double post

[gonzaloriestra]

What about following this other approach? I think it's a bit more readable and it just loops through each file once.

Suggested change

	paths = root.glob(pattern)
	if raise_on_dir
	paths.each do |path|
	if ::File.directory?(path)
	@ctx.abort(@ctx.message('theme.serve.error.invalid_subdirectory', path.to_s))
	end
	end
	end
	paths.map { |path| File.new(path, root) }
	end
	root.glob(pattern).map do |path|
	abort_if_directory(path) if raise_on_dir
	File.new(path, root)
	end
	def abort_if_directory(path)
	return unless ::File.directory?(path)
	@ctx.abort(@ctx.message('theme.serve.error.invalid_subdirectory', path.to_s))
	end

[amcaplan]

Love it, thanks for the suggestion @gonzaloriestra!

[github-actions]

This PR seems inactive. If it's still relevant, please add a comment saying so. Otherwise, take no action.

→ If there's no activity within a week, then a bot will automatically close this.

Thanks for helping to improve Shopify's dev tooling and experience.