Migrate from inliner to web-resource-inliner #4266
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
softhack007
added
dependencies
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThe changes update the dependencies and related inlining code. In package.json, the dependency "inliner" has been removed and replaced with "web-resource-inliner", and the nodemon version has been bumped. In tools/cdata.js, the HTML inlining process has been modified to use the new library’s inline.html method while the overall control flow remains largely unchanged. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Caller
participant WG as writeHtmlGzipped
participant I as inline.html
participant M as Minifier
participant G as GZIP
participant F as File System
C ->> WG: Invoke writeHtmlGzipped(file, relativeTo)
WG ->> I: Call inline.html(fileContent, relativeTo)
I -->> WG: Return inlined HTML
WG ->> M: Minify HTML content
M -->> WG: Return minified HTML
WG ->> G: Compress minified HTML
G -->> WG: Return gzipped data
WG ->> F: Write gzipped output to file
F -->> WG: File write confirmation
WG -->> C: Return success
Assessment against linked issues
✨ Finishing Touches
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
package.json(1 hunks)tools/cdata.js(2 hunks)
🔇 Additional comments (2)
package.json (1)
28-29: Verify the status of web-resource-inliner package.Let's check if the new package is actively maintained and secure.
✅ Verification successful
🌐 Web query:
What is the latest version of web-resource-inliner? When was it last updated? Is it actively maintained?💡 Result:
The latest version of **web-resource-inliner** is **7.0.0**, last updated on **August 17, 2024** according to its npm registry entry[5][9]. This represents a major update from previous versions like v5.x (July 2020)[9] and v4.x (November 2023)[1]. Key maintenance indicators: - The package received TypeScript definition updates through **@types/web-resource-inliner@4.2.5** in November 2023[1] - Recent documentation changes include security updates (XSS vulnerability fixes) and expanded database integration features[4] - The GitHub repository shows ongoing development with recent additions like service worker support and Postgres JDBC driver compatibility[6][4] However, there are no commits or releases documented in the search results after August 2024[5], suggesting development activity has slowed in the last 6 months (as of February 2025). For critical production use, verify compatibility with current Node.js versions and security requirements[6][9]. Citations: - 1: https://classic.yarnpkg.com/en/package/@types/web-resource-inliner - 2: https://carldesouza.com/refreshing-a-web-resource-in-unified-interface-with-javascript-the-new-way/ - 3: https://support.uptime.com/hc/en-us/articles/360016256840-Status-Page-Forms-and-Fields - 4: https://secure.croydon.gov.uk/eforms/doc/Changes_in_this_release.htm - 5: https://www.npmjs.com/package/web-resource-inliner - 6: https://github.com/remy/inliner/ - 7: https://html.spec.whatwg.org/multipage/webappapis.html - 8: https://developers.google.com/apps-script/docs/release-notes?hl=en - 9: https://classic.yarnpkg.com/en/package/web-resource-inliner - 10: https://community.dynamics.com/forums/thread/details/?threadid=8a7e148d-c2e4-47ff-966a-2ca1899b0732
Web-resource-inliner package status verified
- The latest version is 7.0.0, and it was last updated on August 17, 2024.
- Maintenance indicators such as recent TypeScript definition updates (November 2023), security documentation changes (including XSS vulnerability fixes), and new features in the GitHub repository confirm active support.
- Although there have been no commits or releases after August 2024, the overall evidence supports that the package is maintained and secure for production use, with the caveat to verify compatibility with your current Node.js environment.
tools/cdata.js (1)
20-20: LGTM!The module import has been updated to use the new package.
Comment on lines
+131
to
+150
| inline.html({ | ||
| fileContent: fs.readFileSync(sourceFile, "utf8"), | ||
| relativeTo: path.dirname(sourceFile), | ||
| strict: true, | ||
| }, | ||
| async function (error, html) { | ||
| if (error) throw error; | ||
|
|
||
| html = adoptVersionAndRepo(html); | ||
| const originalLength = html.length; | ||
| html = await minify(html, "html-minify"); | ||
| const result = zlib.gzipSync(html, { level: zlib.constants.Z_BEST_COMPRESSION }); | ||
| console.info("Minified and compressed " + sourceFile + " from " + originalLength + " to " + result.length + " bytes"); | ||
| const array = hexdump(result); | ||
| let src = singleHeader; | ||
| src += `const uint16_t PAGE_${page}_L = ${result.length};\n`; | ||
| src += `const uint8_t PAGE_${page}[] PROGMEM = {\n${array}\n};\n\n`; | ||
| console.info("Writing " + resultFile); | ||
| fs.writeFileSync(resultFile, src); | ||
| }); | ||
| html = adoptVersionAndRepo(html); | ||
| const originalLength = html.length; | ||
| html = await minify(html, "html-minify"); | ||
| const result = zlib.gzipSync(html, { level: zlib.constants.Z_BEST_COMPRESSION }); | ||
| console.info("Minified and compressed " + sourceFile + " from " + originalLength + " to " + result.length + " bytes"); | ||
| const array = hexdump(result); | ||
| let src = singleHeader; | ||
| src += `const uint16_t PAGE_${page}_L = ${result.length};\n`; | ||
| src += `const uint8_t PAGE_${page}[] PROGMEM = {\n${array}\n};\n\n`; | ||
| console.info("Writing " + resultFile); | ||
| fs.writeFileSync(resultFile, src); | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify error handling and options configuration.
The migration to web-resource-inliner looks good, but a few suggestions to improve the implementation:
- Consider adding error handling for the file read operation
- The 'strict' option is set but other potentially useful options like 'images', 'svgs' might be worth considering
Consider this improved implementation:
- inline.html({
- fileContent: fs.readFileSync(sourceFile, "utf8"),
- relativeTo: path.dirname(sourceFile),
- strict: true,
- },
+ try {
+ const fileContent = fs.readFileSync(sourceFile, "utf8");
+ inline.html({
+ fileContent,
+ relativeTo: path.dirname(sourceFile),
+ strict: true,
+ images: true,
+ svgs: true,
+ },
async function (error, html) {
if (error) throw error;
// ... rest of the code ...
});
+ } catch (error) {
+ console.error(`Error reading file ${sourceFile}:`, error);
+ throw error;
+ }🏁 Script executed:
#!/bin/bash
# Print lines 120-160 of tools/cdata.js to verify error handling and option configuration
sed -n '120,160p' tools/cdata.jsLength of output: 1579
Inline HTML error handling and option configuration need improvement.
- The file read operation via fs.readFileSync is not wrapped within a try-catch block, meaning any file read errors aren't caught before passing data to inline.html.
- The current inline.html options only include strict mode. Additional options like images and svgs—which could enhance behavior—are missing.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This migrates from the old inliner package to the web-resource-inliner package.
Now we don't get this warning anymore:
If you are wondering why the flash size increased, it is because the old inline package did not respect
/*! these comments */with exclamation marks.Now, if you look at the source code, you can see the iro.js license.
Before:
Now:
This fixes #4166.
Summary by CodeRabbit