refactor: reorganize components folder structure#1
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's GuideRefactors the Astro project’s components into a domain‑oriented folder structure, introduces path aliases and auto-imported UI/content components, and updates all imports, configs, and a few component implementations to match. Class diagram for reorganized Astro components and new Image componentclassDiagram
class BaseLayout_astro {
+string title
+string description
+string? ogImage
}
class BlogPostLayout_astro {
+CollectionEntry blogPost
+MarkdownHeading[] headings
}
class Navbar_astro {
}
class ProgressBar_astro {
}
class BlogPostCard_astro {
+CollectionEntry post
}
class CategoryCard_astro {
+string category
}
class Toc_astro {
+MarkdownHeading[] headings
}
class ViewCounter_astro {
+string slug
+updateViewCount() async
}
class ToolCard_astro {
+string title
+string description
+string href
+string icon
}
class FeatureCard_astro {
+string title
+string description
+string icon
}
class FeatureGrid_astro {
}
class Search_astro {
+string placeholder
}
class InfoBox_astro {
+string title
+string type
}
class CodeExplainer_astro {
+string title
+string language
}
class GitCommand_astro {
+string command
+string description
}
class ProsCons_astro {
}
class Table_astro {
}
class Image_astro {
+string src
+string alt
+string? caption
+number? width
+number? height
+HTMLAttributes figureAttributes
}
class Slugify_ts {
+slugify(text string) string
}
%% Layout composition
BaseLayout_astro --> Navbar_astro : uses
BaseLayout_astro --> ProgressBar_astro : uses
BlogPostLayout_astro --> BaseLayout_astro : wraps
BlogPostLayout_astro --> Toc_astro : uses
BlogPostLayout_astro --> ViewCounter_astro : uses
BlogPostLayout_astro --> Slugify_ts : uses
%% Blog listing and category pages
BlogPostCard_astro --> Slugify_ts : uses
%% Content components used inside MDX
InfoBox_astro --> CodeExplainer_astro : may_coexist
ProsCons_astro --> InfoBox_astro : may_coexist
%% New Image component attributes relationship
Image_astro --> InfoBox_astro : often_coexists_in_content
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The new
ViewCounterstill uses a hard-codedid="view-count-value", which will be duplicated if multiple counters render on the same page; consider scoping the element lookup (e.g. via aref,data-attribute, orthiscontainer) instead of a global ID. - The
Imagecomponent currently forwards extra props only to the<figure>; if you expect callers to pass image-specific attributes (e.g.loading,sizes,srcset,aria-*for the<img>), you may want to split and forward relevant attributes to the<img>element as well for better flexibility. - With the introduction of the
@/*path alias andastro-auto-importconfiguration, it may be worth checking that your non-Astro tooling (ESLint, IDE TypeScript config, tests) are aligned with these aliases and auto-imported components to avoid mismatches between build-time and editor behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `ViewCounter` still uses a hard-coded `id="view-count-value"`, which will be duplicated if multiple counters render on the same page; consider scoping the element lookup (e.g. via a `ref`, `data-` attribute, or `this` container) instead of a global ID.
- The `Image` component currently forwards extra props only to the `<figure>`; if you expect callers to pass image-specific attributes (e.g. `loading`, `sizes`, `srcset`, `aria-*` for the `<img>`), you may want to split and forward relevant attributes to the `<img>` element as well for better flexibility.
- With the introduction of the `@/*` path alias and `astro-auto-import` configuration, it may be worth checking that your non-Astro tooling (ESLint, IDE TypeScript config, tests) are aligned with these aliases and auto-imported components to avoid mismatches between build-time and editor behavior.
## Individual Comments
### Comment 1
<location> `src/components/content/Image.astro:23-32` </location>
<code_context>
+} = Astro.props;
+---
+
+<figure class:list={['image-wrapper', className]} {...attrs}>
+ <img
+ src={src}
+ alt={alt}
+ width={width}
+ height={height}
+ loading="lazy"
+ decoding="async"
+ />
+ {caption && <figcaption>{caption}</figcaption>}
+</figure>
+
</code_context>
<issue_to_address>
**suggestion:** Image-specific attributes like `loading`/`decoding` can’t currently be overridden per-usage.
`loading="lazy"` and `decoding="async"` are hardcoded on `<img>`, and the rest of the passed HTML attributes are applied to `<figure>`, so callers can’t override these or pass `sizes`, `srcset`, or `fetchpriority` to the image. Consider adding an `imgProps` bag, or separating known figure props from image-level props and spreading the latter on `<img>` to allow overrides while keeping the current defaults.
Suggested implementation:
```
class: className,
imgProps = {},
...figureAttrs
} = Astro.props;
---
```
```
<figure class:list={['image-wrapper', className]} {...figureAttrs}>
<img
src={src}
alt={alt}
width={width}
height={height}
{...imgProps}
loading={imgProps?.loading ?? 'lazy'}
decoding={imgProps?.decoding ?? 'async'}
/>
{caption && <figcaption>{caption}</figcaption>}
</figure>
```
```
import type { HTMLAttributes } from 'astro/types';
type Props = {
src: string;
alt: string;
caption?: string;
width?: number;
height?: number;
imgProps?: HTMLAttributes<'img'>;
} & HTMLAttributes<'figure'>;
```
</issue_to_address>
### Comment 2
<location> `src/components/blog/ViewCounter.astro:10-13` </location>
<code_context>
<span id="view-count-value">...</span> views
</span>
-<script define:vars={{ slug }}>
+<script is:inline define:vars={{ slug }}>
const viewCountElement = document.getElementById('view-count-value');
</code_context>
<issue_to_address>
**issue:** Using a fixed `id` in a reusable component can break when multiple instances are rendered.
Here `id="view-count-value"` is shared across all instances, and `document.getElementById` will only ever return the first one. If `ViewCounter` appears multiple times on a page, the others won’t update. Use a per-instance selector instead (e.g. a unique ID derived from `slug`, a `data-*` attribute, or querying relative to a container) so multiple counters work correctly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| <figure class:list={['image-wrapper', className]} {...attrs}> | ||
| <img | ||
| src={src} | ||
| alt={alt} | ||
| width={width} | ||
| height={height} | ||
| loading="lazy" | ||
| decoding="async" | ||
| /> | ||
| {caption && <figcaption>{caption}</figcaption>} |
There was a problem hiding this comment.
suggestion: Image-specific attributes like loading/decoding can’t currently be overridden per-usage.
loading="lazy" and decoding="async" are hardcoded on <img>, and the rest of the passed HTML attributes are applied to <figure>, so callers can’t override these or pass sizes, srcset, or fetchpriority to the image. Consider adding an imgProps bag, or separating known figure props from image-level props and spreading the latter on <img> to allow overrides while keeping the current defaults.
Suggested implementation:
class: className,
imgProps = {},
...figureAttrs
} = Astro.props;
---
<figure class:list={['image-wrapper', className]} {...figureAttrs}>
<img
src={src}
alt={alt}
width={width}
height={height}
{...imgProps}
loading={imgProps?.loading ?? 'lazy'}
decoding={imgProps?.decoding ?? 'async'}
/>
{caption && <figcaption>{caption}</figcaption>}
</figure>
import type { HTMLAttributes } from 'astro/types';
type Props = {
src: string;
alt: string;
caption?: string;
width?: number;
height?: number;
imgProps?: HTMLAttributes<'img'>;
} & HTMLAttributes<'figure'>;
| <span id="view-count-value">...</span> views | ||
| </span> | ||
|
|
||
| <script define:vars={{ slug }}> | ||
| <script is:inline define:vars={{ slug }}> |
There was a problem hiding this comment.
issue: Using a fixed id in a reusable component can break when multiple instances are rendered.
Here id="view-count-value" is shared across all instances, and document.getElementById will only ever return the first one. If ViewCounter appears multiple times on a page, the others won’t update. Use a per-instance selector instead (e.g. a unique ID derived from slug, a data-* attribute, or querying relative to a container) so multiple counters work correctly.
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
refactor Folder Structure in component and fix import paths and type definitions
Summary by Sourcery
Restructure the components folder into domain-specific subfolders and update the project to use path aliases and auto-imported Astro components.
New Features:
Enhancements:
Build: