Skip to content

refactor: replace puppeteer with astral for screenshot and close #2919#2920

Merged
marvinhagemeister merged 5 commits intodenoland:mainfrom
SisyphusZheng:fixscreenshot-problem
May 12, 2025
Merged

refactor: replace puppeteer with astral for screenshot and close #2919#2920
marvinhagemeister merged 5 commits intodenoland:mainfrom
SisyphusZheng:fixscreenshot-problem

Conversation

@SisyphusZheng
Copy link
Contributor

@SisyphusZheng SisyphusZheng commented May 11, 2025

due to resolves the NotFound: No such file or directory error in the www/utils/screenshot.ts script, which was caused by improper file path construction. This error prevented screenshots from being saved correctly. close #2919

-- update --patch1

Current Status

  • Using npm:puppeteer for webpage screenshots
  • npm lifecycle script warnings present
  • deno-puppeteer is no longer maintained
  • npm:puppeteer still has compatibility issues

References

Changes Made

  1. Dependency Replacement

    • Replaced npm:puppeteer with jsr:@astral/astral
    • Removed npm lifecycle script warnings
  2. Dependency Management Optimization

    • Centralized all imports in deno.json
    • Using jsr: and URL imports

Testing Steps

  1. Run screenshot command:
deno task screenshot https://google.com google
  1. Check output:

    • Verify no npm warnings
    • Validate generated image files:
      • ./www/static/showcase/google2x.jpg
      • ./www/static/showcase/google1x.jpg
    • Confirm image quality
  2. Run complete test suite:

deno task ok
  • Ensure all tests pass
  1. Verify functionality:
    • Screenshot feature works correctly
    • Image processing (2x/1x) functions properly

his PR resolves the `NotFound: No such file or directory` error in the `www/utils/screenshot.ts` script, which was caused by improper file path construction. This error prevented screenshots from being saved correctly.
Copy link
Contributor

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should work fine too.

SisyphusZheng and others added 2 commits May 12, 2025 15:30
Co-authored-by: Asher Gomez <ashersaupingomez@gmail.com>
Co-authored-by: Asher Gomez <ashersaupingomez@gmail.com>
@SisyphusZheng
Copy link
Contributor Author

These should work fine too.
LGTM. Thank you for your prompt response and help!

@SisyphusZheng SisyphusZheng requested a review from iuioiua May 12, 2025 07:33
Copy link
Contributor

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please run deno fmt. Also, are the deno.lock file changes needed?

## Current Status
- Using npm:puppeteer for webpage screenshots
- npm lifecycle script warnings present
- deno-puppeteer is no longer maintained
- npm:puppeteer still has compatibility issues

## References
- [astral](https://github.com/lino-levan/astral): Browser automation library designed for Deno
- [deno-puppeteer#92](lucacasonato/deno-puppeteer#92): Recent discussion about deno-puppeteer

## Changes Made
1. Dependency Replacement
   - Replaced npm:puppeteer with jsr:@astral/astral
   - Removed npm lifecycle script warnings

2. Dependency Management Optimization
   - Centralized all imports in deno.json
   - Using jsr: and URL imports

## Testing Steps
1. Run screenshot command:
```bash
deno task screenshot https://google.com google
```

2. Check output:
   - Verify no npm warnings
   - Validate generated image files:
     - ./www/static/showcase/google2x.jpg
     - ./www/static/showcase/google1x.jpg
   - Confirm image quality

3. Run complete test suite:
```bash
deno task ok
```
- Ensure all tests pass

4. Verify functionality:
   - Screenshot feature works correctly
   - Image processing (2x/1x) functions properly
@SisyphusZheng SisyphusZheng requested a review from iuioiua May 12, 2025 09:26
@SisyphusZheng SisyphusZheng changed the title fix: screenshot script #2919 refactor: replace puppeteer with astral for screenshot and close #2919 May 12, 2025
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes aren't needed right now if #2900 is merged soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx, I'll track #2900 and follow up once it's merged.👏

Copy link
Collaborator

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@marvinhagemeister marvinhagemeister merged commit d68b5ec into denoland:main May 12, 2025
4 of 7 checks passed
@SisyphusZheng SisyphusZheng deleted the fixscreenshot-problem branch May 12, 2025 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

screentshot script script can not be work well

3 participants