Skip to content

Create processor in useEffect in MarkdownHooks #908

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

Closed
wants to merge 1 commit into from

Conversation

remcohaszing
Copy link
Member

Initial checklist

  • I read the support docs
  • I read the contributing guide
  • I agree to follow the code of conduct
  • I searched issues and discussions and couldn’t find anything or linked relevant results below
  • I made sure the docs are up to date
  • I included tests (or that’s not needed)

Description of changes

In MarkdownHooks we don’t need to create the processor for every render. We only need it inside the effect.

In `MarkdownHooks` we don’t need to create the processor for every
render. We only need it inside the effect.
@remcohaszing remcohaszing added 🏁 area/perf This affects performance 👶 semver/patch This is a backwards-compatible fix 🦋 type/enhancement This is great to have 🌐 platform/browser This affects browsers 🤞 phase/open Post is being triaged manually labels Apr 18, 2025
Copy link

codecov bot commented Apr 18, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (2245c64) to head (4f8d5d1).
Report is 47 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##              main      #908    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files            3         3            
  Lines         1354      1743   +389     
  Branches       113       123    +10     
==========================================
+ Hits          1354      1743   +389     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JounQin
Copy link
Member

JounQin commented Apr 18, 2025

LGTM. Would it improve perf and resolve https://github.com/orgs/rehypejs/discussions/194?

@remcohaszing
Copy link
Member Author

Not necessarily, but it does give me an idea. I need to fiddle a bit.

@remcohaszing remcohaszing marked this pull request as draft April 18, 2025 18:20
@remcohaszing
Copy link
Member Author

I’m closing this in favor of #909

This comment was marked as resolved.

@remcohaszing remcohaszing deleted the processor-in-effect branch April 18, 2025 18:48
@wooorm wooorm added the 🙅 no/wontfix This is not (enough of) an issue for this project label Apr 19, 2025

This comment was marked as resolved.

@wooorm
Copy link
Member

wooorm commented Apr 19, 2025

please apply labels when closing, thanks!

@github-actions github-actions bot added 👎 phase/no Post cannot or will not be acted on and removed 🤞 phase/open Post is being triaged manually labels Apr 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏁 area/perf This affects performance 🙅 no/wontfix This is not (enough of) an issue for this project 👎 phase/no Post cannot or will not be acted on 🌐 platform/browser This affects browsers 👶 semver/patch This is a backwards-compatible fix 🦋 type/enhancement This is great to have
Development

Successfully merging this pull request may close these issues.

3 participants