Skip to content
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

perf: memoize the extensions in code editor to reduce expensive calculations #441

Merged
merged 1 commit into from
May 21, 2023

Conversation

shanpriyan
Copy link
Contributor

Prerequisites checklist

What is the purpose of this pull request?

Memoize the extensions in code editor to reduce expensive calculations on every re-render

What changes did you make? (Give an overview)

Used useMemo to wrap the extensions array to keep the reference.

Is there anything you'd like reviewers to focus on?

image

Ref: https://www.npmjs.com/package/@uiw/react-codemirror?activeTab=readme

@netlify
Copy link

netlify bot commented May 20, 2023

👷 Deploy request for es-eslint pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit dfaef64

@netlify
Copy link

netlify bot commented May 20, 2023

Deploy Preview for zh-hans-eslint ready!

Name Link
🔨 Latest commit dfaef64
🔍 Latest deploy log https://app.netlify.com/sites/zh-hans-eslint/deploys/6468e356b63c6d0008225ec0
😎 Deploy Preview https://deploy-preview-441--zh-hans-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented May 20, 2023

Deploy Preview for ja-eslint ready!

Name Link
🔨 Latest commit dfaef64
🔍 Latest deploy log https://app.netlify.com/sites/ja-eslint/deploys/6468e35627708e000861da2b
😎 Deploy Preview https://deploy-preview-441--ja-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented May 20, 2023

Deploy Preview for new-eslint ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit dfaef64
🔍 Latest deploy log https://app.netlify.com/sites/new-eslint/deploys/6468e35630cb7a00082dae31
😎 Deploy Preview https://deploy-preview-441--new-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented May 20, 2023

Deploy Preview for fr-eslint ready!

Name Link
🔨 Latest commit dfaef64
🔍 Latest deploy log https://app.netlify.com/sites/fr-eslint/deploys/6468e356632ba80008b83d0c
😎 Deploy Preview https://deploy-preview-441--fr-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented May 20, 2023

Deploy Preview for hi-eslint ready!

Name Link
🔨 Latest commit dfaef64
🔍 Latest deploy log https://app.netlify.com/sites/hi-eslint/deploys/6468e35623b41300085e006d
😎 Deploy Preview https://deploy-preview-441--hi-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented May 20, 2023

Deploy Preview for pt-br-eslint ready!

Name Link
🔨 Latest commit dfaef64
🔍 Latest deploy log https://app.netlify.com/sites/pt-br-eslint/deploys/6468e356ec4a970007d10a6a
😎 Deploy Preview https://deploy-preview-441--pt-br-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented May 20, 2023

Deploy Preview for de-eslint ready!

Name Link
🔨 Latest commit dfaef64
🔍 Latest deploy log https://app.netlify.com/sites/de-eslint/deploys/6468e356d4a7fa0008c69ecb
😎 Deploy Preview https://deploy-preview-441--de-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Member

@amareshsm amareshsm left a comment

Choose a reason for hiding this comment

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

LGTM. I am ok with change. Leaving this open for others to verify/review.

kecrily
kecrily previously approved these changes May 21, 2023
@kecrily kecrily dismissed their stale review May 21, 2023 15:22

The playground of the preview site is broken

@harish-sethuraman
Copy link
Member

Seems to work for me? 🤔 @kecrily

@kecrily
Copy link
Member

kecrily commented May 21, 2023

After some investigation I think this is a problem with the preview component of netlify


Open the preview page, I found an error message at the bottom

image

Open the console, I found that the error report came from under netlify.com.

image

I have also tested locally and everything works fine locally.


@harish-sethuraman Can you verify if my investigation is correct?

@harish-sethuraman
Copy link
Member

These errors do not look like its coming from our app? Its coming from netlify's deploy preview menu in the bottom I guess?

@kecrily
Copy link
Member

kecrily commented May 21, 2023

These errors do not look like its coming from our app? Its coming from netlify's deploy preview menu in the bottom I guess?

Yes, that's what I thought. Just wanted to make sure.

Copy link
Member

@kecrily kecrily left a comment

Choose a reason for hiding this comment

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

LGTM

@kecrily kecrily merged commit c121967 into eslint:main May 21, 2023
@shanpriyan shanpriyan deleted the perf/memo-extension branch May 21, 2023 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants