[Demo][No submit] Dart blog experiment #7079
Conversation
Summary of ChangesHello @antfitch, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request lays the groundwork for a new blog section on the Dart website. It introduces the necessary infrastructure for content loading, asset processing, and dynamic rendering of blog posts. The changes enable authors to easily create and publish blog content with rich media and author information, enhancing the site's ability to share updates and articles with the community. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new blog feature to the site, including dedicated UI components like BlogCard, BlogList, and BlogIndex with associated styling and client-side interactivity for category filtering. It adds a ContentAssetsBuilder to process and copy image assets from src/content to web/images/content, and an ImagePathProcessor to rewrite relative image paths in markdown content. A BlogDataLoader is implemented to load blog post data from markdown files, parse frontmatter, and fetch author information from GitHub, with a FilteredFilesystemLoader replacing the generic FilesystemLoader to specifically handle markdown and HTML files. Review comments highlight issues with inconsistent path handling in the FilesystemLoader's file watcher events, the sequential and potentially slow GitHub API calls in BlogDataLoader for fetching author data, and the limited custom frontmatter parser in BlogDataLoader which should be replaced with a more robust YAML parsing library.
| _watcherSub ??= watcherFactory(directory).events.listen((event) { | ||
| // It looks like event.path is relative on most platforms, but an | ||
| // absolute path on Linux. Turn this into the expected relative path. | ||
| final path = p.normalize(p.relative(event.path)); | ||
| if (event.type == ChangeType.MODIFY) { | ||
| invalidateFile(path); | ||
| } else if (event.type == ChangeType.REMOVE) { | ||
| removeFile(path); | ||
| } else if (event.type == ChangeType.ADD) { | ||
| addFile(path); | ||
| } |
There was a problem hiding this comment.
The path handling for file watcher events appears to have issues with relative versus absolute paths. The removeFile and invalidateFile methods compare the incoming path with source.file.path, which is an absolute path. However, the path generated from a watch event is derived from p.relative(event.path), which likely produces a path relative to the current working directory, not an absolute one. This mismatch will cause hot-reloading of file removals and modifications to fail.
Furthermore, addFile also receives this relative path and attempts to compute a sub-path from it using directory, which can also fail if the paths are not consistently handled.
The entire path resolution logic for watcher events needs to be revisited to ensure that paths are consistently resolved to absolute paths before being used in these methods.
| import 'dart:async'; | ||
|
|
||
| import 'package:build/build.dart'; | ||
| import 'package:path/path.dart' as p; |
There was a problem hiding this comment.
This import is unused and can be removed.
| try { | ||
| // Note: In a real CI/CD environment, you'd want a GITHUB_TOKEN here. | ||
| // For local dev/static sites, unauthenticated requests usually suffice | ||
| // for small volume. | ||
| final url = Uri.parse('https://api.github.com/users/$handle'); | ||
| final response = await http.get( | ||
| url, | ||
| headers: { | ||
| // User-Agent is required by GitHub API | ||
| 'User-Agent': 'Dart-Blog-Loader', | ||
| 'Accept': 'application/vnd.github.v3+json', | ||
| }, | ||
| ); | ||
|
|
||
| if (response.statusCode == 200) { | ||
| final data = jsonDecode(response.body) as Map<String, dynamic>; | ||
| if (data['name'] != null) { | ||
| authorName = data['name'] as String; | ||
| } | ||
| } | ||
| // Cache the result (even if failed, to avoid retrying) | ||
| githubCache[handle] = {'name': authorName}; | ||
| } catch (e) { | ||
| print('Failed to fetch GitHub data for $handle: $e'); | ||
| // Fallback is already set to default or whatever we have | ||
| } | ||
| } |
There was a problem hiding this comment.
The GitHub API is called sequentially within a loop for each new author handle. For a build with many new authors, this will be slow as each network request waits for the previous one to complete. To improve build performance, it would be better to first iterate through all posts to collect the set of unique author handles that need to be fetched, and then execute all network requests in parallel using Future.wait.
| Map<String, String> _parseFrontmatter(String content) { | ||
| final match = RegExp(r'^---\n(.*?)\n---', dotAll: true).firstMatch(content); | ||
| if (match == null) return {}; | ||
|
|
||
| final yamlContent = match.group(1)!; | ||
| final map = <String, String>{}; | ||
|
|
||
| for (final line in yamlContent.split('\n')) { | ||
| final parts = line.split(':'); | ||
| if (parts.length >= 2) { | ||
| final key = parts[0].trim(); | ||
| var value = parts.sublist(1).join(':').trim(); | ||
| if (value.startsWith('"') && value.endsWith('"')) { | ||
| value = value.substring(1, value.length - 1); | ||
| } | ||
| map[key] = value; | ||
| } | ||
| } | ||
| return map; | ||
| } |
There was a problem hiding this comment.
This custom frontmatter parser is limited and only supports simple string key-value pairs. It will fail on more complex but valid YAML, such as lists, nested maps, or multi-line strings. Using a dedicated YAML parsing package like package:yaml would make this implementation more robust, maintainable, and aligned with standard practices for handling frontmatter. This would likely require changing the function's return type to Map<String, dynamic> and updating call sites to handle dynamic values, but would significantly improve the robustness of the blog post loading.
|
Visit the preview URL for this PR (updated for commit e908181): https://dart-dev--pr7079-dart-blog-migration-5pu72gtg.web.app |
2 participants
This is a demo of what our future blog could look like for Dart. Still a WIP.