-
-
Notifications
You must be signed in to change notification settings - Fork 3
add: pnpm-lock.yaml #35
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Reviewer's Guide by SourceryThis pull request adds the pnpm-lock.yaml file to the repository. This file is used by pnpm to ensure that the same versions of dependencies are installed across different environments. This helps to ensure that the application is built and runs consistently. No diagrams generated as the changes look simple and do not need a visual representation. Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThis pull request transitions the project’s dependency management from Yarn to PNPM. Numerous configuration files, CI/CD workflows, Dockerfiles, documentation files, and scripts have been updated accordingly. In addition, new configuration files, CSS stylesheets, and TypeScript declarations have been introduced. The updates also include enhancements to tooling (ESLint, npm, VS Code tasks, webpack) and adjustments to the build and server commands. Overall, the changes modernize and streamline project setup, dependency resolution, and documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant PR as Pull Request
participant GH as GitHub Actions
participant Checkout as Checkout Action
participant Node as Node.js Setup
participant PNPM as PNPM CLI
participant Export as pnpm-lock-export
participant Comment as PR Commenter
PR->>GH: Trigger on dependency change (pnpm-lock.yaml)
GH->>Checkout: Checkout repository code
Checkout->>Node: Set up Node.js (v18)
Node->>PNPM: Install PNPM globally via npm
PNPM->>PNPM: Install dependencies (--frozen-lockfile)
PNPM->>Export: Run pnpm-lock-export for lock changes
Export->>GH: Generate markdown summary of changes
GH->>Comment: Comment on the pull request with summary
Suggested labels
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have skipped reviewing this pull request. Here's why:
- It seems to have been created by a bot (hey, khulnasoft-bot!). We assume it knows what it's doing!
- All of the files are larger than we can process. We're working on it!
✅ Deploy Preview for shipyard ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
CI Feedback 🧐(Feedback updated until commit c20939a)A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
|
Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here. CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
| const fontPath = path.join(outputDir, font.filename); | ||
| await downloadFile(woff2Url, fontPath); | ||
| } catch (err) { | ||
| console.error(`❌ Failed to download ${font.name}:`, err); |
Check warning
Code scanning / CodeQL
Log injection Medium
user-provided value
Log entry depends on a
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 10 months ago
To fix the log injection issue, we need to sanitize the err.message before logging it. This can be done by removing any newline characters from the err.message to prevent log injection attacks. We will use String.prototype.replace to achieve this.
-
Copy modified lines R81-R82
| @@ -80,3 +80,4 @@ | ||
| } catch (err) { | ||
| console.error(`❌ Failed to download ${font.name}:`, err); | ||
| const sanitizedMessage = err.message.replace(/\n|\r/g, ""); | ||
| console.error(`❌ Failed to download ${font.name}: ${sanitizedMessage}`); | ||
| } |
| const weatherIconPath = path.join(widgetDir, weatherIcon.name); | ||
| await downloadFile(weatherIcon.url, weatherIconPath); | ||
| } catch (err) { | ||
| console.error(`❌ Error downloading ${weatherIcon.name}:`, err); |
Check warning
Code scanning / CodeQL
Log injection Medium
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 10 months ago
To fix the log injection issue, we need to sanitize the err.message before logging it. This can be done by removing any newline characters from the error message. We will use String.prototype.replace to achieve this. The changes will be made in the downloadFile function where the error is initially caught and in the downloadAllFonts function where the error is logged.
-
Copy modified lines R37-R38 -
Copy modified lines R101-R102
| @@ -36,3 +36,4 @@ | ||
| fs.unlink(outputPath, () => {}); | ||
| reject(`❌ Error downloading ${url}: ${err.message}`); | ||
| const sanitizedMessage = err.message.replace(/\n|\r/g, ""); | ||
| reject(`❌ Error downloading ${url}: ${sanitizedMessage}`); | ||
| }); | ||
| @@ -99,3 +100,4 @@ | ||
| } catch (err) { | ||
| console.error(`❌ Error downloading ${weatherIcon.name}:`, err); | ||
| const sanitizedMessage = err.message.replace(/\n|\r/g, ""); | ||
| console.error(`❌ Error downloading ${weatherIcon.name}: ${sanitizedMessage}`); | ||
| } |
Signed-off-by: NxPKG <116948796+NxPKG@users.noreply.github.com>
Signed-off-by: NxPKG <116948796+NxPKG@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🧹 Nitpick comments (27)
src/@ (1)
1-1: Clarify the Placeholder File's PurposeThis new file contains only a single dot (
.), which suggests it might be intended as a placeholder to ensure the directory is tracked by Git. It would be helpful to add a descriptive comment (or consider using a more conventional filename like.gitkeep) so that future maintainers immediately understand its purpose.public/css/schema-editor.css (1)
10-11: Remove duplicate font name in font-family declaration- font-family: "Source Code Pro", "SF Mono", Monaco, Inconsolata, "Fira Mono", - "Droid Sans Mono", monospace, monospace; + font-family: "Source Code Pro", "SF Mono", Monaco, Inconsolata, "Fira Mono", + "Droid Sans Mono", monospace;The word "monospace" appears twice in the font-family declaration, which is redundant.
🧰 Tools
🪛 Biome (1.9.4)
[error] 11-11: Duplicate font names are redundant and unnecessary: monospace
Remove duplicate font names within the property
(lint/suspicious/noDuplicateFontNames)
docker/Dockerfile-old (1)
15-15: Consider consistency in package file copying between Dockerfiles.This explicitly copies package.json and pnpm-lock.yaml, while Dockerfile-arm32v7 uses wildcards with
package*.jsonbut doesn't explicitly copy the lock file.Consider using the same approach across all Dockerfiles for consistency:
-COPY package.json pnpm-lock.yaml ./ +COPY package*.json pnpm-lock.yaml ./scripts/update-sass.sh (1)
22-27: Appropriate feedback and SASS command execution.The script provides clear feedback about what it's doing and uses the correct SASS command with proper flags.
Consider adding a check to verify the sass version to ensure compatibility with the compressed style and load path options:
# Check if sass is installed if ! command -v sass &> /dev/null; then echo "sass is not installed. Please install it using: npm install -g sass" exit 1 fi + +# Verify sass version +SASS_VERSION=$(sass --version) +echo "Using Sass version: $SASS_VERSION"scripts/download-fonts.js (4)
42-45: Improve URL encoding for font namesThe current implementation only replaces spaces with '+' characters, but font names might contain other special characters that need URL encoding.
const getGoogleFontsCss = (fontName) => { - const formatted = fontName.replace(/ /g, '+'); + const formatted = encodeURIComponent(fontName).replace(/%20/g, '+'); return `https://fonts.googleapis.com/css2?family=${formatted}&display=swap`; };
47-62: Enhance error handling in HTTP requestThe
fetchCssfunction lacks status code checking and request timeout, which could lead to unexpected behavior if the server returns an error or never responds.const fetchCss = (url) => { return new Promise((resolve, reject) => { const options = { headers: { 'User-Agent': 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36', }, + timeout: 10000, // 10 seconds timeout }; https.get(url, options, (res) => { + if (res.statusCode !== 200) { + return reject(`Failed to fetch CSS: Status code ${res.statusCode}`); + } let data = ''; res.on('data', (chunk) => (data += chunk)); res.on('end', () => resolve(data)); }).on('error', (err) => reject(err)); }); };
85-102: Improve download performance with parallel processingThe current implementation downloads fonts sequentially. Using
Promise.allwould allow for concurrent downloads and improve performance.const downloadAllFonts = async () => { const fontsDir = path.join(__dirname, '..', 'public', 'fonts'); const widgetDir = path.join(__dirname, '..', 'public', 'widget-resources'); fs.mkdirSync(fontsDir, { recursive: true }); fs.mkdirSync(widgetDir, { recursive: true }); - for (const font of fonts) { - await downloadGoogleFont(font, fontsDir); - } + // Download all fonts in parallel + await Promise.all(fonts.map(font => downloadGoogleFont(font, fontsDir))); try { const weatherIconPath = path.join(widgetDir, weatherIcon.name); await downloadFile(weatherIcon.url, weatherIconPath); } catch (err) { console.error(`❌ Error downloading ${weatherIcon.name}:`, err); } };🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 100-100: Log injection
Log entry depends on a user-provided value.
16-19: Document the source of the weather iconAdding a comment about the source of the weather icon would improve maintainability, especially if the URL changes in the future.
+// Weather Icons font from Erik Flowers' weather-icons project +// https://github.com/erikflowers/weather-icons const weatherIcon = { name: 'WeatherIcons.woff2', url: 'https://erikflowers.github.io/weather-icons/font/weathericons-regular-webfont.woff2', };docs/widgets.md (1)
1382-1418: Add Alt Text to the Widget ImageThe new "Minecraft Server" widget section is clear and well structured. One small improvement is needed for accessibility: the image tag on line 1384 is missing an alt attribute. Please add a descriptive alt text (for example,
alt="Minecraft Server status widget") to comply with accessibility guidelines and resolve MD045 warnings.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
1386-1386: Images should have alternate text (alt text)
null(MD045, no-alt-text)
public/css/weather-icons.css (1)
1-1: Format this minified CSS file for better readabilityThe CSS is currently minified into a single line, which makes it difficult to read, maintain, and review. While minification is appropriate for production, the source file in version control should ideally be formatted for readability.
Consider adding a pre-minified version to the repository or using a build step to handle minification:
-@font-face{font-family:"OpenWeatherIcons";src:url("/widget-resources/WeatherIcons.woff2");font-style:normal;font-weight:400}i.owi{display:inline-block;transform:translate(0, 0);text-rendering:auto;font-family:OpenWeatherIcons;font-style:normal;font-size:inherit;-webkit-font-smoothing:antialiased;color:var(--primary)}i.owi.owi-01d::before{content:""}i.owi.owi-01n::before{content:""}i.owi.owi-02d::before{content:""}i.owi.owi-02n::before{content:""}i.owi.owi-03d::before{content:""}i.owi.owi-03n::before{content:""}i.owi.owi-04d::before{content:""}i.owi.owi-04n::before{content:""}i.owi.owi-09d::before{content:""}i.owi.owi-09n::before{content:""}i.owi.owi-10d::before{content:""}i.owi.owi-10n::before{content:""}i.owi.owi-11d::before{content:""}i.owi.owi-11n::before{content:""}i.owi.owi-13d::before{content:""}i.owi.owi-13n::before{content:""}i.owi.owi-50d::before{content:""}i.owi.owi-50n::before{content:""}i.owi.owi-1232n::before{content:""}/*# sourceMappingURL=weather-icons.css.map */ +@font-face { + font-family: "OpenWeatherIcons"; + src: url("/widget-resources/WeatherIcons.woff2"); + font-style: normal; + font-weight: 400; +} + +i.owi { + display: inline-block; + transform: translate(0, 0); + text-rendering: auto; + font-family: OpenWeatherIcons; + font-style: normal; + font-size: inherit; + -webkit-font-smoothing: antialiased; + color: var(--primary); +} + +i.owi.owi-01d::before { content: ""; } +i.owi.owi-01n::before { content: ""; } +i.owi.owi-02d::before { content: ""; } +i.owi.owi-02n::before { content: ""; } +i.owi.owi-03d::before { content: ""; } +i.owi.owi-03n::before { content: ""; } +i.owi.owi-04d::before { content: ""; } +i.owi.owi-04n::before { content: ""; } +i.owi.owi-09d::before { content: ""; } +i.owi.owi-09n::before { content: ""; } +i.owi.owi-10d::before { content: ""; } +i.owi.owi-10n::before { content: ""; } +i.owi.owi-11d::before { content: ""; } +i.owi.owi-11n::before { content: ""; } +i.owi.owi-13d::before { content: ""; } +i.owi.owi-13n::before { content: ""; } +i.owi.owi-50d::before { content: ""; } +i.owi.owi-50n::before { content: ""; } +i.owi.owi-1232n::before { content: ""; } + +/*# sourceMappingURL=weather-icons.css.map */Also, the static analysis hints indicate some parsing issues at the beginning of the file that may need to be checked.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: expected
,but instead found@Remove @
(parse)
[error] 1-1: expected
,but instead foundfont-faceRemove font-face
(parse)
public/css/color-themes.css (2)
1-6: Review Error Message Comment Block.
The lengthy comment (lines 1–6) clearly documents an "Undefined mixin" error. Ensure that this block is intentional for debugging purposes and not left in production styles.
9-10: Remove Duplicate Font Name.
Static analysis flagged a duplicatemonospaceentry in thefont-family. Removing the duplicate will clean up the rule.- font-family: "Source Code Pro", "SF Mono", Monaco, Inconsolata, "Fira Mono", - "Droid Sans Mono", monospace, monospace; + font-family: "Source Code Pro", "SF Mono", Monaco, Inconsolata, "Fira Mono", + "Droid Sans Mono", monospace;🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: Duplicate font names are redundant and unnecessary: monospace
Remove duplicate font names within the property
(lint/suspicious/noDuplicateFontNames)
public/css/style-helpers.css (1)
1-2: CSS Helpers File: Minification and Maintainability.
The new CSS file correctly defines scrollbar, button, text alignment, and utility classes using CSS variables. Although minification is acceptable for production, consider using a formatted version during development to improve readability and ease future modifications.docs/developing.md (4)
27-27: Prerequisites Tone Enhancement.
Consider using a more formal phrasing in the prerequisites section (e.g., "ensure that you have installed" instead of "ensure you've got") to elevate the documentation's tone.🧰 Tools
🪛 LanguageTool
[style] ~27-~27: Try using a more formal synonym here to elevate your writing.
Context: ... the code, and push any changes. If you plan on running or deploying the container, you'll also...(PLAN_ON_INTEND)
42-42: Typo Correction: "Straightforward".
The phrase "this is very straight forward" should be revised to "this is very straightforward" (one word) for improved clarity and correctness.- ...this is very straight forward. Just run `pnpm build` ... + ...this is very straightforward. Just run `pnpm build` ...🧰 Tools
🪛 LanguageTool
[grammar] ~42-~42: It appears that the verb form is incorrect or a word is missing.
Context: ...ld-time, and therefore the app needs to rebuilt for any new changes to take effect. Luc...(HAS_TO_APPROVED_BY)
[typographical] ~42-~42: Consider adding a comma after ‘Luckily’ for more clarity.
Context: ...ilt for any new changes to take effect. Luckily this is very straight forward. Just run...(RB_LY_COMMA)
[uncategorized] ~42-~42: The adjective or adverb “straightforward” (= easy, precise) is spelled as one word.
Context: ...es to take effect. Luckily this is very straight forward. Just runpnpm buildor `docker exec ...(STRAIGHT_FORWARD_COMPOUND)
59-59: Possessive Form Correction.
Consider updating "files within the projects root" to "files within the project's root" for correct grammatical usage.- ...changes to any files within the projects root, and then trigger... + ...changes to any files within the project's root, and then trigger...🧰 Tools
🪛 LanguageTool
[uncategorized] ~59-~59: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...tch for changes to any files within the projects root, and then trigger a rebuild. Note ...(AI_HYDRA_LEO_APOSTROPHE_S_XS)
184-184: Tone Refinement in the Style Guide Section.
In the style guide, consider revising "runpnpm lintto report and fix issues" to a more formal expression (e.g., "executepnpm lintto identify and correct issues") to further enhance the documentation’s formality.🧰 Tools
🪛 LanguageTool
[style] ~184-~184: Consider using a different verb for a more formal wording.
Context: .... You can runpnpm lintto report and fix issues. While the dev server is running...(FIX_RESOLVE)
docs/quick-start.md (1)
138-145: Transition to PNPM for Alternative Deployment
The alternative deployment instructions now use PNPM commands (pnpm install,pnpm build,pnpm start), aligning with the overall shift in package management. Additionally, note the introductory line at 138—consider reviewing the punctuation (for example, adding a comma after[Node.js](https://nodejs.org/)) to improve clarity in the list.🧰 Tools
🪛 LanguageTool
[uncategorized] ~138-~138: Possible missing comma found.
Context: ... app on your system without Docker. For this Git, [Node.js](...(AI_HYDRA_LEO_MISSING_COMMA)
.github/pr-auto-comments.yml (1)
32-35: Updated Dependency File Reference and YAML Formatting
Thedependency-changessnippet now targetspnpm-lock.yaml, which is consistent with the project's switch from Yarn to PNPM. However, a YAML indentation warning was reported (expected 8 spaces but found 6) on this block. Please adjust the indentation to ensure correct parsing. Consider applying the following diff:- files: - - pnpm-lock.yaml + files: + - pnpm-lock.yaml🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 34-34: wrong indentation: expected 8 but found 6
(indentation)
.github/workflows/get-size.yml (1)
15-19: PNPM Installation – Update Deprecated Command
In the PNPM installation step, the command on line 18 currently uses the deprecated::add-path::syntax. Please update it to use the new GitHub Actions command as follows:- echo "::add-path::$(pnpm bin)" + echo "$(pnpm bin)" >> $GITHUB_PATHThis change follows GitHub’s updated best practices for modifying the system PATH in workflows.
🧰 Tools
🪛 actionlint (1.7.4)
16-16: workflow command "add-path" was deprecated. use
echo "{path}" >> $GITHUB_PATHinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
.github/workflows/pnpm.yml (1)
13-13: Upgrade actions/checkout to a Newer Version
Static analysis flags that usingactions/checkout@v2is outdated. Upgrading toactions/checkout@v3would provide improved performance and better security. Consider applying the following diff:- - uses: actions/checkout@v2 + - uses: actions/checkout@v3🧰 Tools
🪛 actionlint (1.7.4)
13-13: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
public/css/dimensions.css (1)
1-2: Introduce CSS Custom Properties for Consistent Styling
The new CSS file defines a comprehensive set of custom properties that streamline styling across the application. The definitions look correct. For improved maintainability and readability, consider formatting these properties with line breaks and optional comments.docs/management.md (2)
772-773: Typographical Improvement Needed
The sentence “Note that the app must have been build (runpnpm build), and you need Node.js installed.” should use the past participle “built” (i.e. “... must have been built ...”) for clarity and correct grammar.🧰 Tools
🪛 LanguageTool
[grammar] ~772-~772: Make sure that the noun ‘build’ is correct. Did you mean the past participle “built”?
Context: ...rver. Note that the app must have been build (runpnpm build`), and you need [Node....(BE_VB_OR_NN)
906-907: Development Workflow Instructions Review
The step-by-step instructions for forking the project, installing dependencies with PNPM (pnpm devto start development, followed bypnpm buildandpnpm start) are clear. Consider a slight rephrasing for brevity if desired.🧰 Tools
🪛 LanguageTool
[style] ~906-~906: Consider shortening or rephrasing this to strengthen your wording.
Context: ...ost:8080` in your browser. You can then make changes to the codebase, and see the live app upda...(MAKE_CHANGES)
docs/troubleshooting.md (1)
146-146: Stylistic Suggestion for Clarity
The phrase “First of all, check that you've got PNPM installed correctly…” could be made more concise by removing the introductory adverb. This small change might improve clarity.🧰 Tools
🪛 LanguageTool
[style] ~146-~146: Often, this adverbial phrase is redundant. Consider using an alternative.
Context: ...ub.com//issues/1)First of all, check that you've got PNPM installed c...
(FIRST_OF_ALL)
vue.config.js (1)
110-135: Consider removing unnecessary polyfills to reduce bundle size.
While these fallbacks ensure Node modules work in the browser, they can inflate the bundle. If not all are needed, removing them could yield performance benefits.public/css/color-palette.css (1)
1-1: Consider formatting color variables for readability.
Currently, all variables are on a single line. Splitting them into multiple lines or grouping logically can improve maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (23)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpublic/css/color-palette.css.mapis excluded by!**/*.mappublic/css/dimensions.css.mapis excluded by!**/*.mappublic/css/global-styles.css.mapis excluded by!**/*.mappublic/css/media-queries.css.mapis excluded by!**/*.mappublic/css/style-helpers.css.mapis excluded by!**/*.mappublic/css/typography.css.mapis excluded by!**/*.mappublic/css/user-defined-themes.css.mapis excluded by!**/*.mappublic/css/weather-icons.css.mapis excluded by!**/*.mappublic/css/widgets/nextcloud-shared.css.mapis excluded by!**/*.mappublic/widget-resources/WeatherIcons.woff2is excluded by!**/*.woff2src/assets/fonts/Audiowide-Regular.ttfis excluded by!**/*.ttfsrc/assets/fonts/CutiveMono-Regular.ttfis excluded by!**/*.ttfsrc/assets/fonts/Digital-Regular.ttfis excluded by!**/*.ttfsrc/assets/fonts/FrancoisOne-Regular.ttfis excluded by!**/*.ttfsrc/assets/fonts/Podkova-Medium.ttfis excluded by!**/*.ttfsrc/assets/fonts/Roboto-Light.ttfis excluded by!**/*.ttfsrc/assets/fonts/Shrikhand-Regular.ttfis excluded by!**/*.ttfsrc/assets/fonts/Sniglet-Regular.ttfis excluded by!**/*.ttfsrc/assets/fonts/VT323-Regular.ttfis excluded by!**/*.ttfsrc/assets/fonts/WeatherIcons.woff2is excluded by!**/*.woff2src/assets/widget-resources/WeatherIcons.woff2is excluded by!**/*.woff2yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (52)
.devcontainer/devcontainer.json(1 hunks).dockerignore(1 hunks).eslintrc.js(1 hunks).github/pr-auto-comments.yml(2 hunks).github/workflows/dependency-updates-summary.yml(1 hunks).github/workflows/docker-build-publish.yml(1 hunks).github/workflows/get-size.yml(3 hunks).github/workflows/pnpm.yml(1 hunks).github/workflows/yarn.yml(0 hunks).gitignore(1 hunks).gitpod.yml(2 hunks).npmrc(1 hunks).vscode/tasks.json(2 hunks)Dockerfile(2 hunks)README.md(4 hunks)docker/Dockerfile-arm32v7(2 hunks)docker/Dockerfile-arm64v8(2 hunks)docker/Dockerfile-lite(1 hunks)docker/Dockerfile-old(1 hunks)docs/configuring.md(2 hunks)docs/credits.md(1 hunks)docs/deployment.md(2 hunks)docs/developing.md(3 hunks)docs/development-guides.md(2 hunks)docs/management.md(5 hunks)docs/quick-start.md(2 hunks)docs/troubleshooting.md(5 hunks)docs/widgets.md(2 hunks)netlify.toml(1 hunks)package.json(3 hunks)pnpm-workspace.yaml(1 hunks)public/css/color-palette.css(1 hunks)public/css/color-themes.css(1 hunks)public/css/dimensions.css(1 hunks)public/css/global-styles.css(1 hunks)public/css/media-queries.css(1 hunks)public/css/schema-editor.css(1 hunks)public/css/style-helpers.css(1 hunks)public/css/typography.css(1 hunks)public/css/user-defined-themes.css(1 hunks)public/css/weather-icons.css(1 hunks)public/css/widgets/nextcloud-shared.css(1 hunks)scripts/download-fonts.js(1 hunks)scripts/update-sass.sh(1 hunks)server.js(1 hunks)services/rebuild-app.js(1 hunks)src/@(1 hunks)src/types/global.d.ts(1 hunks)src/types/index.ts(1 hunks)src/types/vue-shim.d.ts(1 hunks)tsconfig.json(2 hunks)vue.config.js(2 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/yarn.yml
🧰 Additional context used
🧬 Code Definitions (2)
scripts/download-fonts.js (2)
services/rebuild-app.js (1)
require(5-5)vue.config.js (2)
path(1-1)match(161-161)
vue.config.js (2)
services/rebuild-app.js (1)
require(5-5)scripts/download-fonts.js (2)
path(3-3)match(65-65)
🪛 Biome (1.9.4)
public/css/color-themes.css
[error] 10-10: Duplicate font names are redundant and unnecessary: monospace
Remove duplicate font names within the property
(lint/suspicious/noDuplicateFontNames)
public/css/schema-editor.css
[error] 11-11: Duplicate font names are redundant and unnecessary: monospace
Remove duplicate font names within the property
(lint/suspicious/noDuplicateFontNames)
vue.config.js
[error] 24-24: Use a regular expression literal instead of the RegExp constructor.
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
(lint/complexity/useRegexLiterals)
[error] 325-325: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 340-340: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
public/css/weather-icons.css
[error] 1-1: expected , but instead found @
Remove @
(parse)
[error] 1-1: expected , but instead found font-face
Remove font-face
(parse)
🪛 GitHub Actions: Dependency Updates Summary
docker/Dockerfile-lite
[error] 10-10: Error installing octokit: public_suffix requires Ruby version >= 3.0. The current ruby version is 2.7.8.225.
[error] 10-10: Command failed with exit code 1.
docker/Dockerfile-old
[error] 10-10: Error installing octokit: public_suffix requires Ruby version >= 3.0. The current ruby version is 2.7.8.225.
[error] 10-10: Command failed with exit code 1.
docker/Dockerfile-arm32v7
[error] 10-10: Error installing octokit: public_suffix requires Ruby version >= 3.0. The current ruby version is 2.7.8.225.
[error] 10-10: Command failed with exit code 1.
Dockerfile
[error] 10-10: Error installing octokit: public_suffix requires Ruby version >= 3.0. The current ruby version is 2.7.8.225.
[error] 10-10: Command failed with exit code 1.
docker/Dockerfile-arm64v8
[error] 10-10: Error installing octokit: public_suffix requires Ruby version >= 3.0. The current ruby version is 2.7.8.225.
[error] 10-10: Command failed with exit code 1.
🪛 LanguageTool
docs/quick-start.md
[uncategorized] ~138-~138: Possible missing comma found.
Context: ... app on your system without Docker. For this Git, [Node.js](...
(AI_HYDRA_LEO_MISSING_COMMA)
docs/developing.md
[style] ~27-~27: Try using a more formal synonym here to elevate your writing.
Context: ... the code, and push any changes. If you plan on running or deploying the container, you'll also...
(PLAN_ON_INTEND)
[style] ~36-~36: Consider shortening or rephrasing this to strengthen your wording.
Context: ...lhost:8080/>. Hot reload is enabled, so making changes to any of the files will trigger them to b...
(MAKE_CHANGES)
[grammar] ~42-~42: It appears that the verb form is incorrect or a word is missing.
Context: ...ld-time, and therefore the app needs to rebuilt for any new changes to take effect. Luc...
(HAS_TO_APPROVED_BY)
[typographical] ~42-~42: Consider adding a comma after ‘Luckily’ for more clarity.
Context: ...ilt for any new changes to take effect. Luckily this is very straight forward. Just run...
(RB_LY_COMMA)
[uncategorized] ~42-~42: The adjective or adverb “straightforward” (= easy, precise) is spelled as one word.
Context: ...es to take effect. Luckily this is very straight forward. Just run pnpm build or `docker exec ...
(STRAIGHT_FORWARD_COMPOUND)
[uncategorized] ~59-~59: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...tch for changes to any files within the projects root, and then trigger a rebuild. Note ...
(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[style] ~184-~184: Consider using a different verb for a more formal wording.
Context: .... You can run pnpm lint to report and fix issues. While the dev server is running...
(FIX_RESOLVE)
docs/management.md
[uncategorized] ~48-~48: Possible missing article found.
Context: ...docker exec -it [container-id], where container ID can be found by running docker ps....
(AI_HYDRA_LEO_MISSING_THE)
[typographical] ~48-~48: After the expression ‘for example’ a comma is usually used.
Context: ...an be found by running docker ps. For example `docker exec -it 26c156c467b4 pnpm buil...
(COMMA_FOR_EXAMPLE)
[uncategorized] ~73-~73: Possible missing article found.
Context: ...e configured to periodically check that Shipyard is up and running correctly on the spec...
(AI_HYDRA_LEO_MISSING_THE)
[grammar] ~772-~772: Make sure that the noun ‘build’ is correct. Did you mean the past participle “built”?
Context: ...rver. Note that the app must have been build (run pnpm build`), and you need [Node....
(BE_VB_OR_NN)
[style] ~906-~906: Consider shortening or rephrasing this to strengthen your wording.
Context: ...ost:8080` in your browser. You can then make changes to the codebase, and see the live app upda...
(MAKE_CHANGES)
docs/troubleshooting.md
[style] ~146-~146: Often, this adverbial phrase is redundant. Consider using an alternative.
Context: ...ub.com//issues/1)
First of all, check that you've got PNPM installed c...
(FIRST_OF_ALL)
🪛 actionlint (1.7.4)
.github/workflows/get-size.yml
12-12: shellcheck reported issue in this script: SC2086:info:1:27: Double quote to prevent globbing and word splitting
(shellcheck)
16-16: workflow command "add-path" was deprecated. use echo "{path}" >> $GITHUB_PATH instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
.github/workflows/pnpm.yml
13-13: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/dependency-updates-summary.yml
15-15: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
20-20: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 GitHub Check: CodeQL
scripts/download-fonts.js
[warning] 81-81: Log injection
Log entry depends on a user-provided value.
Log entry depends on a user-provided value.
[warning] 100-100: Log injection
Log entry depends on a user-provided value.
🪛 markdownlint-cli2 (0.17.2)
docs/management.md
67-67: Link fragments should be valid
null
(MD051, link-fragments)
docs/widgets.md
1386-1386: Images should have alternate text (alt text)
null
(MD045, no-alt-text)
🪛 YAMLlint (1.35.1)
.github/pr-auto-comments.yml
[warning] 34-34: wrong indentation: expected 8 but found 6
(indentation)
🔇 Additional comments (97)
public/css/media-queries.css (1)
1-1: LGTM: Added source map for improved debuggingAdding the source map URL helps developers trace compiled CSS back to its original source, enabling better debugging in browser developer tools.
public/css/user-defined-themes.css (1)
1-1: LGTM: Added source map for improved debuggingAdding the source map URL helps developers trace compiled CSS back to its original source, enabling better debugging in browser developer tools.
server.js (1)
5-5: LGTM: Updated comment to reference pnpmComment has been correctly updated to reflect the transition from Yarn to pnpm.
.dockerignore (1)
4-4: LGTM: Updated Docker requirements to reference pnpm-lock.yamlThe comment has been correctly updated to reference the new pnpm-lock.yaml file instead of yarn.lock.
netlify.toml (2)
8-8: Good update to use pnpm instead of yarn for build commandThe change from
yarn buildtopnpm buildcorrectly aligns with the PR's objective to transition from Yarn to PNPM as the package manager.
16-17: Correctly configured Netlify for PNPM supportThe changes correctly configure Netlify to use PNPM by:
- Replacing
YARN_FLAGSwithNPM_FLAGS="--version"- Adding
ENABLE_PNPM="true"which is required for Netlify to recognize and use PNPMThese environment variables are essential for proper PNPM functionality in the Netlify build environment.
.gitpod.yml (2)
6-7: Successful migration from yarn to pnpm for initializationThe updates correctly change both the initialization and startup commands from using Yarn to PNPM, maintaining the same flags like
--ignore-engines. This is consistent with the PR objective to transition to PNPM.
25-25: Fixed spacing in extensions listThe spacing has been corrected for the GitLens extension entry, which improves the consistency of the YAML formatting.
docker/Dockerfile-lite (2)
9-10: Updated Docker build process for PNPMThe changes correctly update the Docker build process to use PNPM by:
- Copying the
pnpm-lock.yamlinstead ofyarn.lock- Installing PNPM globally using npm
- Using
pnpm install --frozen-lockfilewhich is the equivalent of Yarn's frozen installationThis ensures consistent dependency installation in Docker builds.
🧰 Tools
🪛 GitHub Actions: Dependency Updates Summary
[error] 10-10: Error installing octokit: public_suffix requires Ruby version >= 3.0. The current ruby version is 2.7.8.225.
[error] 10-10: Command failed with exit code 1.
12-12: Updated build command to use PNPMThe build command has been correctly updated from
yarn buildtopnpm buildto complete the migration to PNPM.docker/Dockerfile-old (3)
11-12: PNPM installation added correctly.The global installation of PNPM is correctly implemented, ensuring the package manager is available for subsequent commands.
18-18: LGTM - Correctly using the frozen lockfile flag.The
--frozen-lockfileflag ensures dependency versions are locked, equivalent to Yarn's similar option.
24-24: Commands properly updated to use PNPM.All build, start, and health check commands have been correctly updated to use PNPM instead of Yarn.
Also applies to: 30-30, 33-33
docker/Dockerfile-arm32v7 (2)
13-15: Node.js and PNPM installation split correctly.The approach of installing Node.js and npm first, then PNPM globally is correct.
35-35: Commands correctly updated to use PNPM.All build, start, and health check commands have been properly migrated from Yarn to PNPM.
Also applies to: 41-41, 44-44
scripts/update-sass.sh (2)
3-7: Good error handling for sass installation check.The script correctly checks for the sass command and provides a helpful error message with installation instructions.
13-20: Directory creation and symlink handling is well implemented.The script properly creates directories if they don't exist and sets up the symlink for @ to point to the src directory, which is useful for SASS imports.
.gitignore (2)
11-12: PNPM-specific entries added correctly.The additions of
pnpm-debug.log*and.pnpm-store/align well with the transition from Yarn to PNPM.
19-19: Good additions for common development artifacts.Adding common editor settings, build outputs, logs, and temporary files to .gitignore is a good practice. These additions make the .gitignore file more comprehensive.
Also applies to: 26-34
tsconfig.json (3)
3-4: TypeScript incremental build configuration addedAdding incremental build support with the cache file in node_modules/.cache is a good performance optimization. This will speed up TypeScript compilation by only recompiling files that have changed since the last build.
35-38: Extended TypeScript file patterns to include additional file typesThe include patterns have been expanded to include
.tsx,.js, and.jsxfiles, which ensures proper TypeScript processing for all relevant frontend files in the codebase as part of the migration to PNPM.
41-42: Added 'dist' to exclusion patternsAdding 'dist' to the exclude list is appropriate as it prevents TypeScript from processing compiled output files, avoiding potential circular compilation issues.
.eslintrc.js (1)
1-42: Well-structured ESLint configuration for Vue/TypeScript projectThe ESLint configuration is well organized with appropriate extends, rules, and TypeScript-specific overrides. The ignore patterns will prevent linting of generated files and node_modules.
A few notes:
- The configuration properly extends Vue, Standard, and Airbnb base configs
- TypeScript files are handled with the appropriate parser and plugins
- The ignore patterns and rules are sensible for a Vue/TypeScript project
This configuration aligns well with the transition from Yarn to PNPM and provides consistent linting across the codebase.
docs/development-guides.md (2)
168-170: Documentation updated to reference PNPM instead of YarnThe development guide has been appropriately updated to reference PNPM commands instead of Yarn, and includes proper getting started instructions using the new package manager.
264-264: Widget development prerequisites updated for PNPMThe widget development prerequisites section has been correctly updated to use PNPM instead of Yarn, maintaining consistency with the package manager transition.
docker/Dockerfile-arm64v8 (5)
13-14: Updated Node.js installation to include PNPMThe Docker image now correctly installs PNPM globally using npm, replacing the previous Yarn installation. This change aligns with the project's transition to PNPM as the package manager.
24-25: Updated file copying for PNPM lock fileThe file copy command has been modified to copy
pnpm-lock.yamlinstead ofpackage-lock.json, correctly reflecting the change in package manager.
28-28: Updated dependency installation to use PNPMThe dependency installation command has been updated to use
pnpm install --frozen-lockfile, which is the PNPM equivalent of Yarn's install with frozen lockfile, ensuring reproducible builds.
34-34: Updated build, start, and health check commands to use PNPMAll npm scripts execution commands have been updated to use PNPM instead of Yarn, maintaining consistency with the package manager transition across the entire build and deployment process.
Also applies to: 40-40, 43-43
1-44:Details
❓ Verification inconclusive
Verify Ruby dependency in GitHub Actions
There's a pipeline failure related to Ruby version requirements when installing octokit. This appears to be in the GitHub Actions workflow for dependency updates summary and is not directly related to the Dockerfile changes.
🏁 Script executed:
#!/bin/bash # Check GitHub workflows for Ruby version specifications grep -r "ruby-version" .github/workflows/ || echo "No Ruby version specified in workflows" # Check for Ruby version in GitHub actions find .github/workflows -name "*.yml" -exec cat {} \; | grep -i "ruby"Length of output: 201
Action Required: Investigate and Fix Ruby Version Configuration in GitHub Actions
The changes in the Dockerfile appear unrelated to the pipeline failure. Our verification confirms that no Ruby version is set in any .github/workflows files. The reported failure with installing octokit likely stems from the dependency update workflow not specifying a Ruby version. Please manually verify and update that workflow (or related configuration) accordingly to resolve the Ruby dependency issue.
- The Dockerfile changes are not causing the failure.
- No Ruby version specifications were found in our search of the workflows.
- Address the Ruby configuration in the dependency update GitHub Action separately.
🧰 Tools
🪛 GitHub Actions: Dependency Updates Summary
[error] 10-10: Error installing octokit: public_suffix requires Ruby version >= 3.0. The current ruby version is 2.7.8.225.
[error] 10-10: Command failed with exit code 1.
scripts/download-fonts.js (3)
81-81: Address log injection security warningThe error logging includes unsanitized user input, which could potentially be exploited for log injection attacks.
- console.error(`❌ Failed to download ${font.name}:`, err); + console.error(`❌ Failed to download ${font.name.replace(/[\r\n]/g, '')}:`, err);🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 81-81: Log injection
Log entry depends on a user-provided value.
Log entry depends on a user-provided value.
100-100: Address log injection security warningThe error logging includes unsanitized user input, which could potentially be exploited for log injection attacks.
- console.error(`❌ Error downloading ${weatherIcon.name}:`, err); + console.error(`❌ Error downloading ${weatherIcon.name.replace(/[\r\n]/g, '')}:`, err);🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 100-100: Log injection
Log entry depends on a user-provided value.
1-3: LGTM: Well-structured importsThe imports are clean and well-organized, using only the necessary Node.js built-in modules.
src/types/vue-shim.d.ts (1)
1-4: LGTM: Proper Vue TypeScript declarationThis declaration file correctly defines the
.vuemodule type for TypeScript, ensuring that TypeScript will recognize Vue single-file components when they are imported. This is a standard declaration pattern used in Vue+TypeScript projects.pnpm-workspace.yaml (1)
1-3: LGTM: Proper PNPM workspace configurationThe workspace configuration is correctly set up to include both the root package and all packages under the
srcdirectory. This aligns with the PR's objective of transitioning to PNPM for dependency management.src/types/global.d.ts (1)
1-11: LGTM: Well-structured global TypeScript declarations fileThis global declaration file is well structured with:
- Clear comments explaining its purpose
- Proper extension of the Window interface for future additions
- The empty export to ensure it's treated as a module
This approach follows TypeScript best practices for declaring global types.
public/css/weather-icons.css (1)
1-1:Details
✅ Verification successful
Verify the font resource path is correct
The CSS references a font at
/widget-resources/WeatherIcons.woff2. Please ensure this path is correct in the project structure and that the font file is available at this location.
🏁 Script executed:
#!/bin/bash # Check if the font file exists or if there are references to this path fd -t f "WeatherIcons.woff2" || grep -r "widget-resources/WeatherIcons.woff2" --include="*.{js,ts,html,css}" .Length of output: 154
Font resource path verified
The font file is correctly referenced by the CSS. The file exists at
public/widget-resources/WeatherIcons.woff2, which perfectly aligns with the/widget-resources/WeatherIcons.woff2path used in the CSS.
- Verified file location:
public/widget-resources/WeatherIcons.woff2🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: expected
,but instead found@Remove @
(parse)
[error] 1-1: expected
,but instead foundfont-faceRemove font-face
(parse)
src/types/index.ts (3)
1-2: Clean TypeScript Type Definitions.
The file introduces clear and concise type definitions that lay a solid foundation for type-safety.
3-9: Well-defined User Interface.
TheUserinterface is straightforward and correctly markscreated_atas optional.
11-16: Good AppSettings Structure.
TheAppSettingstype alias enforces a clear contract for theme, language, and notifications settings. The use of a union for the theme property adds clarity and reduces errors..devcontainer/devcontainer.json (1)
26-27: Switch to PNPM in Post-Create Command.
The update replaces the yarn-based installation with PNPM. The use of options like--frozen-lockfile,--no-cache,--network-timeout, and--network-concurrencyis appropriate for ensuring consistent dependency installation.public/css/widgets/nextcloud-shared.css (1)
1-2: New Widget CSS File Looks Solid.
The styles for.nextcloud-widgetare concise and leverage CSS variables effectively. If human readability is a concern, consider formatting the CSS during development while keeping the minified version for production.docs/credits.md (1)
195-200: Update Dependency Change Export Action.
The credits file now reflects the project's transition to PNPM by removing the old yarn-based action and adding thepnpm-lock-exportentry by @unjs. This change is consistent with the overall dependency management update.🧰 Tools
🪛 LanguageTool
[grammar] ~198-~198: It appears that a hyphen is missing in the plural noun “to-dos”?
Context: ...e-action) by @alstr - Opens issues from todos in the code - [pnpm-lock-export](http...(TO_DO_HYPHEN)
.vscode/tasks.json (2)
5-9: Task Configuration Update for PNPM.
The update correctly transitions from an npm script to a shell task that invokes PNPM with the "dev" argument. This change is in line with the project’s move toward using PNPM for dependency management and development.
10-19: Problem Matcher Formatting.
The trailing comma added to the "problemMatcher" array improves JSON consistency. No further changes are needed here.README.md (2)
129-136: Deployment Instructions: PNPM Command Updates.
The "Deploying from Source" section now clearly instructs users to install dependencies, build, and run the application using PNPM commands (pnpm install,pnpm build, andpnpm start). This update ensures consistency with the transition away from Yarn.
545-554: Development Setup Instructions Revised.
The development section now correctly advises installing dependencies and starting the development server with PNPM. This update aligns with the overall change to PNPM and will help new contributors follow a unified setup process.public/css/typography.css (1)
1-2: Typography CSS File Addition.
The new typography.css file defines several @font-face rules and sets up CSS custom properties for body, headings, and monospace fonts. Verify that the referenced font files exist at the specified paths to ensure consistent styling across environments.docs/developing.md (4)
33-35: PNPM Installation Instructions Verification.
The updated instructions for installing dependencies, building, and starting the dev server using PNPM are clear and correct. This change reinforces the move away from Yarn in the project's setup guidelines.
47-49: Development Command Updates.
The commands forpnpm dev,pnpm lint, andpnpm testare correctly documented. This update ensures that contributors use the appropriate PNPM commands throughout the development workflow.
53-55: Utility Commands Update.
The commands for validating configuration (pnpm validate-config) and checking application health (pnpm health-check) now correctly reference PNPM. The inclusion of Docker-based execution instructions further enhances clarity.
58-61: Alternate Start Commands Updated.
The documentation for alternate start commands such aspnpm build-and-start,pnpm build-watch, andpnpm pm2-startis accurate and provides users with various options for running the application.🧰 Tools
🪛 LanguageTool
[uncategorized] ~59-~59: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...tch for changes to any files within the projects root, and then trigger a rebuild. Note ...(AI_HYDRA_LEO_APOSTROPHE_S_XS)
docs/quick-start.md (1)
103-103: Update PNPM Command for Config Validation
The command at line 103 now usespnpm validate-configinstead of the previous validator. Please verify that this correctly validates the configuration per your intended workflow..npmrc (1)
1-8: PNPM Configuration Settings Updated
The new.npmrcfile includes settings such asshamefully-hoist, disabling strict peer dependencies, and specifying Node.js version 18.19.0. These settings appear consistent with PNPM best practices. Please confirm that they align with your project’s dependency management requirements..github/workflows/docker-build-publish.yml (1)
23-25: Environment Variable Update: DH_IMAGE
The environment variableDH_IMAGEwas updated to use${{ secrets.DOCKER_USERNAME }}instead of${{ secrets.DOCKER_REPO }}. Please verify that this change is intentional—ensuring that the Docker image is now identified by the username—and that all subsequent workflow steps correctly reference this new variable..github/pr-auto-comments.yml (1)
89-94: Updated Ignored Dependencies Notice
The snippet underignored-dependenciesnow indicates that dependencies will be pulled with PNPM (i.e., "when pnpm is run"). This update is in line with the overall changes. Please ensure that any related documentation or scripts referring to Yarn are also updated.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 90-90: wrong indentation: expected 8 but found 6
(indentation)
[error] 94-94: trailing spaces
(trailing-spaces)
.github/workflows/get-size.yml (2)
10-13: Set SHELL Environment Variable Step
The new step setting theSHELLenvironment variable to/bin/bashis clear and should help ensure consistency for shell commands in later steps.🧰 Tools
🪛 actionlint (1.7.4)
12-12: shellcheck reported issue in this script: SC2086:info:1:27: Double quote to prevent globbing and word splitting
(shellcheck)
50-53: Dependency Installation with PNPM
The final step usingpnpm install --frozen-lockfileensures that dependency installations are consistent with the lockfile. This update appears appropriate and effective.docs/deployment.md (2)
182-189: Update Build-from-Source Instructions to Use PNPM
The instructions now correctly instruct users to clone the repository and use PNPM for installing dependencies, building, and running the application. This update is consistent with the overall migration from Yarn to PNPM.
327-329: Revise Surge.sh Deployment Commands
The Surge.sh deployment section now uses PNPM commands (i.e.pnpm add -g surgeandpnpm build) instead of Yarn. This change aligns with the repository-wide shift to PNPM and should ensure consistency in dependency management.docs/configuring.md (1)
19-21: Replace Yarn Commands with PNPM Commands
The “Tips” section has been updated to usepnpm validate-configandpnpm buildinstead of the deprecated Yarn commands. This change is correct and reinforces the transition to PNPM throughout the documentation.🧰 Tools
🪛 LanguageTool
[uncategorized] ~21-~21: You might be missing the article “the” here.
Context: ... config file. You can download it under Config menu, or use the [Cloud Backup](./docs/...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
Dockerfile (4)
18-20: Switch to PNPM for Installing Dependencies
The Dockerfile now copies bothpackage.jsonandpnpm-lock.yaml, and installs dependencies usingpnpm installwith appropriate flags. This ensures consistency with the PNPM migration.
25-25: Update the Build Command for Production
The build command has been updated to usepnpm build --mode production --no-clean, which is in line with the new packaging workflow. Verify that this build step produces the desired output.
45-45: Update the Start Command
The command to start the application now usespnpm build-and-start, reflecting the migration from Yarn. Ensure that the corresponding script in your package configuration is adapted for PNPM if necessary.
51-51: Revise Healthcheck Command
The HEALTHCHECK instruction has been updated to usepnpm health-check. This change is appropriate and aligns with the dependency management strategy.docs/management.md (5)
48-48: Update Docker Command Examples to Use PNPM
The command examples now correctly instruct users to use PNPM (e.g.pnpm buildandpnpm health-check) when running commands in a Docker container.🧰 Tools
🪛 LanguageTool
[uncategorized] ~48-~48: Possible missing article found.
Context: ...docker exec -it [container-id], where container ID can be found by runningdocker ps....(AI_HYDRA_LEO_MISSING_THE)
[typographical] ~48-~48: After the expression ‘for example’ a comma is usually used.
Context: ...an be found by runningdocker ps. For example `docker exec -it 26c156c467b4 pnpm buil...(COMMA_FOR_EXAMPLE)
50-50: Reference to Package Commands Updated Correctly
The text now shows that all available commands are run viapnpm [command-name](using thepackage.jsonas reference), which is in line with the migration from Yarn to PNPM.
52-53: Rebuild Command Updated
The guidance on triggering a rebuild through the UI or via command line now clearly suggests usingpnpm build, ensuring consistency in dependency handling.
54-55: Healthcheck Command Aligned with PNPM
The section now correctly demonstrates runningpnpm health-check(along with the optional--no-healthcheckflag), which confirms the transition to PNPM-based commands.
155-156: Update Instructions for Updating from Source
The updated instructions now tell users to pull the latest code, update dependencies with PNPM, rebuild, and restart—ensuring a smooth transition away from Yarn.public/css/global-styles.css (1)
1-2: Global Styles CSS File Added Successfully
This new CSS file defines a comprehensive set of global styles using CSS variables for consistency across UI components. The selectors and responsive modalities (including custom scrollbars, button states, modal overlays, tooltips, etc.) are well organized.docs/troubleshooting.md (5)
14-14: Table of Contents Updated for PNPM
The entry has been updated to “PNPM Error”, replacing previous Yarn-related references to align with the new dependency manager.
143-154: PNPM Error Section Updated Correctly
This section now provides clear guidelines for troubleshooting dependency errors with PNPM—such as checking your PNPM installation, clearing the cache withpnpm store prune, and reinstalling dependencies. The commands have been updated from Yarn to PNPM consistently.🧰 Tools
🪛 LanguageTool
[style] ~146-~146: Often, this adverbial phrase is redundant. Consider using an alternative.
Context: ...ub.com//issues/1)First of all, check that you've got PNPM installed c...
(FIRST_OF_ALL)
348-349: Config Validation Command Updated
The instruction now suggests usingpnpm validate-configinstead of the old Yarn command, ensuring that the configuration validation step aligns with the PNPM workflow.
356-360: Node-sass Rebuild Command Corrected
The troubleshooting steps now rightly instruct users to runpnpm rebuild node-sassto resolve compatibility issues with node-sass, matching the new dependency management tool.🧰 Tools
🪛 LanguageTool
[grammar] ~358-~358: This phrase is duplicated. You should probably use “for a” only once.
Context: ...sed by node-sass's binaries being built for a for a different architecture
To fix this, ju...(PHRASE_REPETITION)
373-377: Dependency Cache Clearing Steps Aligned with PNPM
The updated instructions now recommend clearing the cache withpnpm store pruneand then reinstalling dependencies, which is both clear and consistent with the PNPM migration..github/workflows/dependency-updates-summary.yml (4)
1-7: Workflow Trigger Updated to Monitor PNPM Lockfile
The event trigger now listens specifically for changes topnpm-lock.yaml(instead ofyarn.lock), which ensures that dependency updates are monitored accurately in the new workflow.
15-15: Action Version Review for Checkout Step
The workflow usesactions/checkout@v3. Please verify whether a newer version is available to address any runner-related issues; if so, consider an update.🧰 Tools
🪛 actionlint (1.7.4)
15-15: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
20-20: Action Version Review for Setup-Node Step
The workflow specifiesactions/setup-node@v3with Node 18. It would be worthwhile to check if an updated version is available to benefit from the latest improvements.🧰 Tools
🪛 actionlint (1.7.4)
20-20: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
30-36: PR Comment Step Configured Correctly
The final step successfully generates a markdown summary of PNPM lockfile changes and posts it as a comment on the PR, thereby streamlining dependency reviews.vue.config.js (11)
3-4: New imports for Webpack plugins look good.
No concerns—importingwebpackandcompression-webpack-pluginis standard practice for advanced Webpack setups.
56-74: Adding Node polyfills and compression plugin is appropriate.
These additions align with modern Webpack 5 apps. The gzip compression for production builds is a solid choice.
78-78: Conditional devtool settings are well-structured.
Switching to'eval-source-map'in development improves rebuild speed and'source-map'in production helps debugging.
86-101: Verify “transpileOnly” usage in ts-loader.
Running intranspileOnlymode does not check types during build. Ensure you have a separate “type-check” script or CI step so that type errors don’t slip through.
136-172: Configuration for externals, output, and chunk splitting looks solid.
No issues spotted—this is a typical approach to improving caching and performance.
177-181: DevServer configuration is aligned with Webpack 5 defaults.
Serving static content, enabling HMR, and specifying custom headers is well-handled.
200-203: Page configuration updates seem correct.
Renaming or defining the “index” entry and associating the correct template is standard practice.
217-218: Refactored PWA property assignment is consistent.
PassingpwaConfigdirectly is succinct and maintainable.
221-223: filenameHashing & productionSourceMap options are standard.
Enabling hashed filenames for cache-busting and disabling full source maps in production is typical.
224-231: Merging configureWebpack correctly.
The spread operator approach maintains clarity by consolidating rules into a single object.
234-236: Conditional CSS extraction meets production needs.
Extracting CSS in production helps with caching; no issues found.package.json (6)
10-11: Verify the decision to skip PWA plugins in build scripts.
Line 10 (“--skip-plugins pwa”) removes certain PWA features. Confirm whether you plan to maintain PWA functionality separately.
25-27: Including “dependency-audit” script is commendable.
This ensures vulnerabilities can be quickly identified.
30-89: Check potential conflicts between AWS SDK v2 and v3.
You have"aws-sdk": "^2.1.0"alongside"@aws-sdk/client-s3": "^3.0.0". Running both can lead to increased bundle sizes or version mismatches.
193-198: lint-staged rules for JS, Vue, and TS ensure consistency.
Automating checks with “eslint --fix” and “prettier --write” is a good development practice.
199-203: commitlint config is set up correctly.
Helps standardize commit messages and keeps the project consistent.
204-205: Locked to pnpm@8.15.4 is consistent with your new setup.
This ensures everyone uses the same version of PNPM across all environments.
|
|
||
| module.exports = () => new Promise((resolve, reject) => { | ||
| const buildProcess = exec('npm run build'); // Trigger the build command | ||
| const buildProcess = exec('NODE_OPTIONS="--max-old-space-size=512" npm run build'); // Trigger the build command |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Inconsistent package manager usage detected
The command here uses npm run build while other parts of the codebase have been updated to use pnpm. Consider updating this to use pnpm for consistency.
Additionally, the NODE_OPTIONS flag to limit memory is a good addition for improving build stability in resource-constrained environments.
🏁 Script executed:
#!/bin/bash
# Check if there are other npm commands still in use in the codebase
echo "Searching for npm commands in JavaScript files..."
rg "npm (run|install)" --type js
echo "-------------------"
echo "Searching for pnpm references for comparison..."
rg "pnpm" --type jsLength of output: 540
Action Required: Update Package Manager Command for Consistency
The search confirms that the services/rebuild-app.js file still uses npm run build while other parts of the codebase (e.g., the note in server.js) reference pnpm build. Please update the command in the rebuild script to use pnpm for consistency. For example, change:
const buildProcess = exec('NODE_OPTIONS="--max-old-space-size=512" npm run build');to
const buildProcess = exec('NODE_OPTIONS="--max-old-space-size=512" pnpm build');The NODE_OPTIONS flag is correctly used to manage memory limits and can remain as is.
| /* Error: Undefined mixin. | ||
| * , | ||
| * 22 | / @include tablet-down { | ||
| * 23 | | flex-direction: column-reverse; | ||
| * 24 | \ } | ||
| * ' | ||
| * src/styles/schema-editor.scss 22:7 root stylesheet */ | ||
|
|
||
| body::before { | ||
| font-family: "Source Code Pro", "SF Mono", Monaco, Inconsolata, "Fira Mono", | ||
| "Droid Sans Mono", monospace, monospace; | ||
| white-space: pre; | ||
| display: block; | ||
| padding: 1em; | ||
| margin-bottom: 1em; | ||
| border-bottom: 2px solid black; | ||
| content: "Error: Undefined mixin.\a \2577 \a 22 \2502 \250c @include tablet-down {\a 23 \2502 \2502 flex-direction: column-reverse;\a 24 \2502 \2514 }\a \2575 \a src/styles/schema-editor.scss 22:7 root stylesheet"; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Review CSS error display implementation
This new CSS file displays a compilation error from a SCSS file. The error indicates an undefined mixin tablet-down in the schema-editor.scss file.
This appears to be an error message that was committed accidentally. The file should be removed or the actual compiled CSS should be committed instead. If this is an intentional error display for development, it should be documented as such.
🏁 Script executed:
#!/bin/bash
# Check if the SCSS mixin is defined elsewhere in the project
echo "Checking for tablet-down mixin definition..."
rg -l "mixin tablet-down" --type scssLength of output: 167
Action Required: Verify Accidental CSS Error Display
It appears that public/css/schema-editor.css is committing a CSS snippet that contains a SCSS compilation error indicating an undefined mixin tablet-down. Our initial attempt to find this mixin using a file type–restricted search resulted in an error due to unsupported file type detection. Please run the above script to search for any SCSS definitions of tablet-down.
- If the mixin is not defined anywhere in the codebase, then this file likely contains an accidental error message and should be removed or replaced with the proper compiled CSS.
- If the error message is intended for development purposes, please document this behavior clearly.
---
🏁 Script executed:
```shell
#!/bin/bash
echo "Searching for SCSS mixin definition for tablet-down..."
rg -l '@mixin tablet-down' --type-add 'scss:*.scss' .
Length of output: 328
Critical: Accidental Error Message in Committed CSS
It appears that the compiled CSS file at public/css/schema-editor.css contains an error message generated by a SCSS compilation failure. Although the mixin tablet-down is defined in ./src/styles/media-queries.scss, it isn’t available in the context of src/styles/schema-editor.scss (likely due to a missing import or misconfiguration). This indicates that instead of committing properly compiled CSS, an error output was accidentally committed.
- Action Items:
- Confirm Intent: If this error display is unintentional, remove
public/css/schema-editor.cssfrom the commit and ensure that the SCSS compiles correctly (possibly by importing the mixin definition fromsrc/styles/media-queries.scssintoschema-editor.scss). - Documentation: If the error display is meant for development purposes, add clear documentation to justify this behavior.
- Confirm Intent: If this error display is unintentional, remove
🧰 Tools
🪛 Biome (1.9.4)
[error] 11-11: Duplicate font names are redundant and unnecessary: monospace
Remove duplicate font names within the property
(lint/suspicious/noDuplicateFontNames)
| WORKDIR ${DIRECTORY} | ||
|
|
||
| # Copy over both 'package.json' and 'package-lock.json' (if available) | ||
| COPY package*.json ./ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing pnpm-lock.yaml and --frozen-lockfile flag.
Two issues that should be addressed:
- The pnpm-lock.yaml file is not explicitly copied
- The install command is missing the --frozen-lockfile flag used in Dockerfile-old
Apply these changes for consistency with Dockerfile-old:
-COPY package*.json ./
+COPY package*.json pnpm-lock.yaml ./
-RUN pnpm install
+RUN pnpm install --frozen-lockfileWithout copying the lock file and using the frozen flag, this Docker build might use different dependency versions than intended.
Also applies to: 29-29
| # local env files | ||
| .env.local | ||
| .env.*.local | ||
| .env |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Consider .env file handling approach.
Adding .env to .gitignore is common practice, but consider if you should provide a template file (like .env.example) to document required environment variables.
🏁 Script executed:
#!/bin/bash
# Check if an .env.example file already exists
if [ ! -f ".env.example" ]; then
echo "No .env.example file found. Consider creating one to document required environment variables."
fiLength of output: 226
Action Required: Add a .env.example Template
The .gitignore update correctly ignores the .env file. However, the verification confirms that a corresponding .env.example file is missing. To improve clarity and assist new developers, please add a .env.example file documenting all required environment variables.
.envis now ignored, which is appropriate.- No
.env.examplefile was found—this file would serve as a template for required variables.
| // Default PWA configuration | ||
| const pwaConfig = { | ||
| name: 'Shipyard', | ||
| themeColor: '#4DBA87', | ||
| msTileColor: '#000000', | ||
| appleMobileWebAppCapable: 'yes', | ||
| appleMobileWebAppStatusBarStyle: 'black', | ||
| workboxPluginMode: 'GenerateSW', | ||
| workboxOptions: { | ||
| navigateFallback: '/index.html', | ||
| runtimeCaching: [ | ||
| { | ||
| urlPattern: new RegExp('^https://'), | ||
| handler: 'NetworkFirst', | ||
| options: { | ||
| networkTimeoutSeconds: 20, | ||
| cacheName: 'https-calls', | ||
| cacheableResponse: { | ||
| statuses: [0, 200], | ||
| }, | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Use a regex literal for improved clarity and alignment with static analysis.
Line 24 uses new RegExp('^https://'). Switching to a literal (/^https:\/\//) is generally clearer and avoids escaping pitfalls.
- urlPattern: new RegExp('^https://'),
+ urlPattern: /^https:\/\//📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Default PWA configuration | |
| const pwaConfig = { | |
| name: 'Shipyard', | |
| themeColor: '#4DBA87', | |
| msTileColor: '#000000', | |
| appleMobileWebAppCapable: 'yes', | |
| appleMobileWebAppStatusBarStyle: 'black', | |
| workboxPluginMode: 'GenerateSW', | |
| workboxOptions: { | |
| navigateFallback: '/index.html', | |
| runtimeCaching: [ | |
| { | |
| urlPattern: new RegExp('^https://'), | |
| handler: 'NetworkFirst', | |
| options: { | |
| networkTimeoutSeconds: 20, | |
| cacheName: 'https-calls', | |
| cacheableResponse: { | |
| statuses: [0, 200], | |
| }, | |
| }, | |
| }, | |
| ], | |
| }, | |
| }; | |
| // Default PWA configuration | |
| const pwaConfig = { | |
| name: 'Shipyard', | |
| themeColor: '#4DBA87', | |
| msTileColor: '#000000', | |
| appleMobileWebAppCapable: 'yes', | |
| appleMobileWebAppStatusBarStyle: 'black', | |
| workboxPluginMode: 'GenerateSW', | |
| workboxOptions: { | |
| navigateFallback: '/index.html', | |
| runtimeCaching: [ | |
| { | |
| urlPattern: /^https:\/\//, | |
| handler: 'NetworkFirst', | |
| options: { | |
| networkTimeoutSeconds: 20, | |
| cacheName: 'https-calls', | |
| cacheableResponse: { | |
| statuses: [0, 200], | |
| }, | |
| }, | |
| }, | |
| ], | |
| }, | |
| }; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 24-24: Use a regular expression literal instead of the RegExp constructor.
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
(lint/complexity/useRegexLiterals)
| config.module.rules.delete('svg'); | ||
|
|
||
| // Enable filesystem caching | ||
| config.cache({ | ||
| type: 'filesystem', | ||
| buildDependencies: { | ||
| config: [__filename], | ||
| }, | ||
| compression: 'gzip', | ||
| maxAge: 86400000, | ||
| name: mode === 'production' ? 'production-cache' : 'development-cache', | ||
| version: process.env.VUE_APP_VERSION || '1.0.0', | ||
| }); | ||
|
|
||
| // Handle SVG files | ||
| config.module | ||
| .rule('svg') | ||
| .test(/\.svg$/) | ||
| .oneOf('inline') | ||
| .resourceQuery(/inline/) | ||
| .use('vue-svg-loader') | ||
| .loader('vue-svg-loader') | ||
| .end() | ||
| .end() | ||
| .oneOf('external') | ||
| .type('asset/resource') | ||
| .set('generator', { | ||
| filename: 'img/[name].[hash:8][ext]', | ||
| }); | ||
|
|
||
| // Configure CSS loader | ||
| const cssRule = config.module.rule('css'); | ||
| const sassRule = config.module.rule('scss'); | ||
| const lessRule = config.module.rule('less'); | ||
| const stylRule = config.module.rule('stylus'); | ||
|
|
||
| const addStyleResource = (rule) => { | ||
| rule.oneOf('vue-modules').resourceQuery(/module/) | ||
| .use('css-loader') | ||
| .tap(options => ({ ...options, url: false })); | ||
|
|
||
| rule.oneOf('vue').use('css-loader') | ||
| .tap(options => ({ ...options, url: false })); | ||
|
|
||
| rule.oneOf('normal-modules').test(/\.module\.\w+$/) | ||
| .use('css-loader') | ||
| .tap(options => ({ ...options, url: false })); | ||
|
|
||
| rule.oneOf('normal').use('css-loader') | ||
| .tap(options => ({ ...options, url: false })); | ||
| }; | ||
|
|
||
| addStyleResource(cssRule); | ||
| addStyleResource(sassRule); | ||
| addStyleResource(lessRule); | ||
| addStyleResource(stylRule); | ||
|
|
||
| // Enable minification in production | ||
| if (mode === 'production') { | ||
| config.optimization.minimize(true); | ||
| } | ||
|
|
||
| // Handle HTML processing to prevent legacy assets issue | ||
| config.plugin('html-index').tap((args) => { | ||
| const newArgs = [...args]; | ||
| if (newArgs[0]) { | ||
| newArgs[0].minify = mode === 'production' ? { | ||
| removeComments: true, | ||
| collapseWhitespace: true, | ||
| removeAttributeQuotes: false, | ||
| collapseBooleanAttributes: true, | ||
| removeScriptTypeAttributes: true, | ||
| } : false; | ||
| // Ensure proper asset handling | ||
| newArgs[0].inject = true; | ||
| newArgs[0].chunksSortMode = 'auto'; | ||
| } | ||
| return newArgs; | ||
| }); | ||
| // Ensure modern mode is disabled to prevent legacy assets issues | ||
| // Get all plugin names | ||
| const pluginNames = Object.keys(config.plugins.entries() || {}); | ||
|
|
||
| // Find and remove any legacy/modern plugins by name | ||
| pluginNames.forEach(name => { | ||
| const plugin = config.plugins.get(name); | ||
| if (plugin && plugin.constructor && plugin.constructor.name | ||
| && (plugin.constructor.name.includes('Modern') | ||
| || plugin.constructor.name.includes('Legacy'))) { | ||
| config.plugins.delete(name); | ||
| } | ||
| }); | ||
|
|
||
| // Override any legacy asset plugins that might be added later in the build process | ||
| const originalProcessWebpackConfig = config.toConfig; | ||
| // eslint-disable-next-line func-names, no-param-reassign | ||
| config.toConfig = function () { | ||
| const webpackConfig = originalProcessWebpackConfig.apply(this); | ||
| // Ensure no legacy/modern plugins are in the final config | ||
| if (webpackConfig.plugins) { | ||
| // eslint-disable-next-line max-len | ||
| webpackConfig.plugins = webpackConfig.plugins.filter(plugin => !(plugin && plugin.constructor && plugin.constructor.name && (plugin.constructor.name.includes('Modern') || plugin.constructor.name.includes('Legacy')))); | ||
| } | ||
| return webpackConfig; | ||
| }; | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Use optional chaining to simplify nested plugin checks.
At lines 324–330 and in the filtering at line 340, you can replace repeated checks with optional chaining for clarity. For instance:
- if (plugin && plugin.constructor && plugin.constructor.name
- && (plugin.constructor.name.includes('Modern')
- || plugin.constructor.name.includes('Legacy'))) {
+ if (plugin?.constructor?.name?.includes('Modern')
+ || plugin?.constructor?.name?.includes('Legacy')) {
config.plugins.delete(name);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Remove default SVG rule to avoid conflicts | |
| config.module.rules.delete('svg'); | |
| // Enable filesystem caching | |
| config.cache({ | |
| type: 'filesystem', | |
| buildDependencies: { | |
| config: [__filename], | |
| }, | |
| compression: 'gzip', | |
| maxAge: 86400000, | |
| name: mode === 'production' ? 'production-cache' : 'development-cache', | |
| version: process.env.VUE_APP_VERSION || '1.0.0', | |
| }); | |
| // Handle SVG files | |
| config.module | |
| .rule('svg') | |
| .test(/\.svg$/) | |
| .oneOf('inline') | |
| .resourceQuery(/inline/) | |
| .use('vue-svg-loader') | |
| .loader('vue-svg-loader') | |
| .end() | |
| .end() | |
| .oneOf('external') | |
| .type('asset/resource') | |
| .set('generator', { | |
| filename: 'img/[name].[hash:8][ext]', | |
| }); | |
| // Configure CSS loader | |
| const cssRule = config.module.rule('css'); | |
| const sassRule = config.module.rule('scss'); | |
| const lessRule = config.module.rule('less'); | |
| const stylRule = config.module.rule('stylus'); | |
| const addStyleResource = (rule) => { | |
| rule.oneOf('vue-modules').resourceQuery(/module/) | |
| .use('css-loader') | |
| .tap(options => ({ ...options, url: false })); | |
| rule.oneOf('vue').use('css-loader') | |
| .tap(options => ({ ...options, url: false })); | |
| rule.oneOf('normal-modules').test(/\.module\.\w+$/) | |
| .use('css-loader') | |
| .tap(options => ({ ...options, url: false })); | |
| rule.oneOf('normal').use('css-loader') | |
| .tap(options => ({ ...options, url: false })); | |
| }; | |
| addStyleResource(cssRule); | |
| addStyleResource(sassRule); | |
| addStyleResource(lessRule); | |
| addStyleResource(stylRule); | |
| // Enable minification in production | |
| if (mode === 'production') { | |
| config.optimization.minimize(true); | |
| } | |
| // Handle HTML processing to prevent legacy assets issue | |
| config.plugin('html-index').tap((args) => { | |
| const newArgs = [...args]; | |
| if (newArgs[0]) { | |
| newArgs[0].minify = mode === 'production' ? { | |
| removeComments: true, | |
| collapseWhitespace: true, | |
| removeAttributeQuotes: false, | |
| collapseBooleanAttributes: true, | |
| removeScriptTypeAttributes: true, | |
| } : false; | |
| // Ensure proper asset handling | |
| newArgs[0].inject = true; | |
| newArgs[0].chunksSortMode = 'auto'; | |
| } | |
| return newArgs; | |
| }); | |
| // Ensure modern mode is disabled to prevent legacy assets issues | |
| // Get all plugin names | |
| const pluginNames = Object.keys(config.plugins.entries() || {}); | |
| // Find and remove any legacy/modern plugins by name | |
| pluginNames.forEach(name => { | |
| const plugin = config.plugins.get(name); | |
| if (plugin && plugin.constructor && plugin.constructor.name | |
| && (plugin.constructor.name.includes('Modern') | |
| || plugin.constructor.name.includes('Legacy'))) { | |
| config.plugins.delete(name); | |
| } | |
| }); | |
| // Override any legacy asset plugins that might be added later in the build process | |
| const originalProcessWebpackConfig = config.toConfig; | |
| // eslint-disable-next-line func-names, no-param-reassign | |
| config.toConfig = function () { | |
| const webpackConfig = originalProcessWebpackConfig.apply(this); | |
| // Ensure no legacy/modern plugins are in the final config | |
| if (webpackConfig.plugins) { | |
| // eslint-disable-next-line max-len | |
| webpackConfig.plugins = webpackConfig.plugins.filter(plugin => !(plugin && plugin.constructor && plugin.constructor.name && (plugin.constructor.name.includes('Modern') || plugin.constructor.name.includes('Legacy')))); | |
| } | |
| return webpackConfig; | |
| }; | |
| }, | |
| // Remove default SVG rule to avoid conflicts | |
| config.module.rules.delete('svg'); | |
| // Enable filesystem caching | |
| config.cache({ | |
| type: 'filesystem', | |
| buildDependencies: { | |
| config: [__filename], | |
| }, | |
| compression: 'gzip', | |
| maxAge: 86400000, | |
| name: mode === 'production' ? 'production-cache' : 'development-cache', | |
| version: process.env.VUE_APP_VERSION || '1.0.0', | |
| }); | |
| // Handle SVG files | |
| config.module | |
| .rule('svg') | |
| .test(/\.svg$/) | |
| .oneOf('inline') | |
| .resourceQuery(/inline/) | |
| .use('vue-svg-loader') | |
| .loader('vue-svg-loader') | |
| .end() | |
| .end() | |
| .oneOf('external') | |
| .type('asset/resource') | |
| .set('generator', { | |
| filename: 'img/[name].[hash:8][ext]', | |
| }); | |
| // Configure CSS loader | |
| const cssRule = config.module.rule('css'); | |
| const sassRule = config.module.rule('scss'); | |
| const lessRule = config.module.rule('less'); | |
| const stylRule = config.module.rule('stylus'); | |
| const addStyleResource = (rule) => { | |
| rule.oneOf('vue-modules').resourceQuery(/module/) | |
| .use('css-loader') | |
| .tap(options => ({ ...options, url: false })); | |
| rule.oneOf('vue').use('css-loader') | |
| .tap(options => ({ ...options, url: false })); | |
| rule.oneOf('normal-modules').test(/\.module\.\w+$/) | |
| .use('css-loader') | |
| .tap(options => ({ ...options, url: false })); | |
| rule.oneOf('normal').use('css-loader') | |
| .tap(options => ({ ...options, url: false })); | |
| }; | |
| addStyleResource(cssRule); | |
| addStyleResource(sassRule); | |
| addStyleResource(lessRule); | |
| addStyleResource(stylRule); | |
| // Enable minification in production | |
| if (mode === 'production') { | |
| config.optimization.minimize(true); | |
| } | |
| // Handle HTML processing to prevent legacy assets issue | |
| config.plugin('html-index').tap((args) => { | |
| const newArgs = [...args]; | |
| if (newArgs[0]) { | |
| newArgs[0].minify = mode === 'production' ? { | |
| removeComments: true, | |
| collapseWhitespace: true, | |
| removeAttributeQuotes: false, | |
| collapseBooleanAttributes: true, | |
| removeScriptTypeAttributes: true, | |
| } : false; | |
| // Ensure proper asset handling | |
| newArgs[0].inject = true; | |
| newArgs[0].chunksSortMode = 'auto'; | |
| } | |
| return newArgs; | |
| }); | |
| // Ensure modern mode is disabled to prevent legacy assets issues | |
| // Get all plugin names | |
| const pluginNames = Object.keys(config.plugins.entries() || {}); | |
| // Find and remove any legacy/modern plugins by name | |
| pluginNames.forEach(name => { | |
| const plugin = config.plugins.get(name); | |
| if (plugin?.constructor?.name?.includes('Modern') | |
| || plugin?.constructor?.name?.includes('Legacy')) { | |
| config.plugins.delete(name); | |
| } | |
| }); | |
| // Override any legacy asset plugins that might be added later in the build process | |
| const originalProcessWebpackConfig = config.toConfig; | |
| // eslint-disable-next-line func-names, no-param-reassign | |
| config.toConfig = function () { | |
| const webpackConfig = originalProcessWebpackConfig.apply(this); | |
| // Ensure no legacy/modern plugins are in the final config | |
| if (webpackConfig.plugins) { | |
| // eslint-disable-next-line max-len | |
| webpackConfig.plugins = webpackConfig.plugins.filter(plugin => !(plugin && plugin.constructor && plugin.constructor.name && (plugin.constructor.name.includes('Modern') || plugin.constructor.name.includes('Legacy')))); | |
| } | |
| return webpackConfig; | |
| }; | |
| }, |
🧰 Tools
🪛 Biome (1.9.4)
[error] 325-325: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 340-340: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
| "ignorePatterns": [ | ||
| "**/node_modules/**", | ||
| "**/dist/**", | ||
| "**/@/**" | ||
| ], | ||
| "rules": { | ||
| "import/no-unresolved": "off", | ||
| "import/extensions": "off", | ||
| "arrow-parens": 0, | ||
| "no-else-return": 0 | ||
| "no-else-return": 0, | ||
| "prefer-regex-literals": "off" | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Re-evaluate turning off “prefer-regex-literals”.
Static analysis flagged the preference for using regex literals, which conflicts with this ESLint override. Consider aligning them to avoid future confusion.
* fix: get-size.yml Signed-off-by: NxPKG <116948796+NxPKG@users.noreply.github.com> * Update get-size.yml Signed-off-by: NxPKG <116948796+NxPKG@users.noreply.github.com> * Update get-size.yml Signed-off-by: NxPKG <116948796+NxPKG@users.noreply.github.com> --------- Signed-off-by: NxPKG <116948796+NxPKG@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/get-size.yml (1)
20-26: Setup Node.js Action – Version Consideration:
The Node.js setup step usingactions/setup-node@v3is correctly configured; however, static analysis indicates that the runner for this action might be too old. It is recommended to update the action version (for example, toactions/setup-node@v3.8or the newest stable release) to address potential compatibility issues.Suggested diff:
- uses: actions/setup-node@v3 + uses: actions/setup-node@v3.8🧰 Tools
🪛 actionlint (1.7.4)
22-22: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/get-size.yml(3 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/get-size.yml
22-22: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (3)
.github/workflows/get-size.yml (3)
10-13: SHELL Environment Variable Setup:
The new step setting the SHELL environment variable to/bin/bashis clear and ensures that subsequent shell commands run with consistent behavior.
14-19: PnPM Setup Step:
The addition of the step to install pnpm usingpnpm/action-setup@v2with thelatestversion is appropriate for ensuring consistent dependency management. Verify that the latest version meets your project’s compatibility requirements.
57-59: Dependency Installation with pnpm:
The step executingpnpm install --frozen-lockfileeffectively enforces consistency with your dependency lockfile. This change aligns well with the overall transition to pnpm.
Category:
Overview
Issue Number (if applicable) #00
New Vars (if applicable)
Screenshot (if applicable)
Code Quality Checklist (Please complete)
Summary by Sourcery
Build:
Summary by CodeRabbit
New Features
Style
Documentation