Repository navigation
fix: remove missing video player icon assets and fail build on downlo… - #668
Merged
Merged
Conversation
…ad errors The build downloaded dist/fonts/cloudinary_icon_for_*.svg from unpkg, but those files don't exist for any player version, so the 404 response body was written to disk as the SVGs. The package exports also pointed to dist/cloudinary_icon_*.svg, which never existed (4 publint errors). Remove the icon downloads and their export paths (the icons are inlined in the player CSS), and make copy-assets fail when a download returns a non-200 response instead of writing the error page as the asset.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Open
This branch was successfully deployed
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Description
Follow-up to my review comment on #665.
During the build,
scripts/copy-assets.tsdownloads two icon files from unpkg (dist/fonts/cloudinary_icon_for_black_bg.svganddist/fonts/cloudinary_icon_for_white_bg.svg). These files don't exist for any player version, and the script didn't check the response, so it saved unpkg's "Not found" page as the SVG files. The published 6.19.3 package ships these files too: example.On
beta,package.jsonalso exports these icons atdist/cloudinary_icon_*.svg, a path where they are never written, which publint reports as 4 errors.This pull request:
exportsand frompreconstruct.exports.extra.downloadFilefail the build when a download doesn't return 200 (or hits a network error), instead of saving the error page as the asset. This protects the player CSS from the same problem.These exports only existed on
betaand never pointed to real files, so this isn't a breaking change.Issue Ticket Number
No separate issue. Raised in the #665 review.
Type of change
Checklist
Testing
dist/contains onlycld-video-player.cssplus the JavaScript and type files, with nofonts/folder.Failed to download … HTTP 404and exit code 1, and no file is written.PLAYER_VERSIONline in the same file).