Skip to content

fix(ideal-image): do not pass down img prop#8250

Merged
Josh-Cena merged 2 commits into
facebook:mainfrom
lex111:lex111/ideal-props
Oct 26, 2022
Merged

fix(ideal-image): do not pass down img prop#8250
Josh-Cena merged 2 commits into
facebook:mainfrom
lex111:lex111/ideal-props

Conversation

@lex111

@lex111 lex111 commented Oct 26, 2022

Copy link
Copy Markdown
Contributor

Pre-flight checklist

  • I have read the Contributing Guidelines on pull requests.
  • If this is a code change: I have written unit tests and/or added dogfooding pages to fully verify the new behavior.
  • If this is a new API or substantial change: the PR has an accompanying issue (closes #0000) and the maintainers have approved on my working plan.

Motivation

Screenshot below speaks for itself:

image

Test Plan

Preview

Test links

Deploy preview: https://deploy-preview-_____--docusaurus-2.netlify.app/

Related issues/PRs

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Oct 26, 2022
@Josh-Cena

Josh-Cena commented Oct 26, 2022

Copy link
Copy Markdown
Collaborator

In this case we don't need to destructure className or alt since they aren't overridden

@Josh-Cena Josh-Cena added the pr: bug fix This PR fixes a bug in a past release. label Oct 26, 2022
@netlify

netlify Bot commented Oct 26, 2022

Copy link
Copy Markdown

✅ [V2]

Built without sensitive environment variables

Name Link
🔨 Latest commit 90acd46
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-2/deploys/6359694d2399970008353bb1
😎 Deploy Preview https://deploy-preview-8250--docusaurus-2.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.

@github-actions

github-actions Bot commented Oct 26, 2022

Copy link
Copy Markdown

⚡️ Lighthouse report for the deploy preview of this PR

URL Performance Accessibility Best Practices SEO PWA Report
/ 🟠 59 🟢 97 🟢 100 🟢 100 🟢 90 Report
/docs/installation 🟠 79 🟢 100 🟢 100 🟢 100 🟢 90 Report

@slorber slorber added the to backport This PR is planned to be backported to a stable version of Docusaurus label Oct 26, 2022
@slorber

slorber commented Oct 26, 2022

Copy link
Copy Markdown
Collaborator

LGTM

Couldn't reproduce the extra img attribute on prod, is this only happening in dev mode?

@Josh-Cena

Josh-Cena commented Oct 26, 2022

Copy link
Copy Markdown
Collaborator

We only use a plain <img/> tag in dev. In prod the actual ideal image component takes care of the prop.

@Josh-Cena Josh-Cena merged commit 258ecf7 into facebook:main Oct 26, 2022
@lex111

lex111 commented Oct 26, 2022

Copy link
Copy Markdown
Contributor Author

We only use a plain tag in dev.

This is also used in the production site see

docusaurus/website/src/pages/showcase/_components/ShowcaseCard/index.tsx

Line 64 in 97c9710

`https://slorber-api-screenshot.netlify.app/${encodeURIComponent(

@slorber

slorber commented Oct 26, 2022

Copy link
Copy Markdown
Collaborator

ah I see, only the ones with a serverless image, can repro now thanks 👍

CleanShot 2022-10-26 at 20 04 28@2x

slorber pushed a commit that referenced this pull request Oct 28, 2022
@lex111 @slorber
fix(ideal-image): do not pass down img prop (#8250)
4c964c6
@slorber slorber added backported This PR has been backported to a stable version of Docusaurus and removed to backport This PR is planned to be backported to a stable version of Docusaurus labels Nov 2, 2022
@slorber slorber mentioned this pull request Oct 19, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Josh-Cena Josh-Cena Josh-Cena approved these changes

@slorber slorber Awaiting requested review from slorber slorber is a code owner

Assignees

No one assigned

Labels

backported This PR has been backported to a stable version of Docusaurus CLA Signed Signed Facebook CLA pr: bug fix This PR fixes a bug in a past release.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@lex111 @Josh-Cena @slorber @facebook-github-bot