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

Image Component doesn't forwardsRef #18398

Closed
piotrzaborow opened this issue Oct 28, 2020 · 33 comments · Fixed by #26824, #32623, #40326 or #43193
Closed

Image Component doesn't forwardsRef #18398

piotrzaborow opened this issue Oct 28, 2020 · 33 comments · Fixed by #26824, #32623, #40326 or #43193
Assignees
Milestone

Comments

@piotrzaborow
Copy link

Bug report

Describe the bug

Refs aren't properly supported in Image.

import React, { useState, useCallback } from 'react'
import Image from 'next/image'

const NextImage = ({ src }) => {
    const [width, setWidth] = useState(0)
    const [height, setHeight] = useState(0)

    const ref = useCallback((node) => {
        console.log(node)
    }, [])

    return <Image ref={ref} src={src} width={width} height={height} />
}

This code shows that Image does not forwardRef to img DOM element.

To Reproduce

  1. Add Image component to your page
  2. Add ref via useCallback hook
  3. Try to do something within callback function

Expected behavior

Image component should pass ref function to img element.

System information

  • OS: macOS
  • Browser (if applies) [Chrome, Safari]
  • Version of Next.js: 10.0.0
  • Version of Node.js: 15.0.0
@Timer Timer added this to the iteration 11 milestone Oct 28, 2020
@Timer Timer added the point: 3 label Oct 30, 2020
@Timer
Copy link
Member

Timer commented Oct 30, 2020

Would you expect this to go to the top-level wrapper or the <img /> component? Could you please explain the use-case and what you're trying to do?

@Timer Timer modified the milestones: iteration 11, 10.x.x Oct 30, 2020
@piotrzaborow
Copy link
Author

I'm building an Image Component based that on Next/Image. I want it set width and height based on it's parent. I had this working on classic img.

I am using useCallback to get parent's dimensions and Set it as a state to use in Next/Image.

@yesmeck
Copy link
Contributor

yesmeck commented Nov 7, 2020

My use-case is adding a link to the image, the Link passes ref to it's child component.

import Image from 'next/image';
import Link from 'next/link';

<Link href="/">
  <Image src="logo.png" />
</Link>

@Timer
Copy link
Member

Timer commented Nov 7, 2020

I'm building an Image Component based that on Next/Image. I want it set width and height based on it's parent. I had this working on classic img.

You do not need a ref for this behavior. The Image component already works like this with its intrinsic behavior.

@piotrzaborow
Copy link
Author

piotrzaborow commented Nov 9, 2020

Two things:

  1. I want to use image transformations in the cloud (my cms provider has an API for this)
  2. The thing that I want to do should work with
  • 'intrinsic': it still needs a width and height property and in my opinion image should take the dimensions of parent and try to scale to it.

  • 'fill' layout: There's a bug in my opinion.

I tried with code written below:

<div style={{width:'20px', height:'20px'}}>
  <Image src='image.src' layout='fill' />
</div>

Original image dimension is 4000px by 4000px. Image should respect parent's width and height when used layout ='fill', but it doesn't work as the documentation describes it. Image is on the whole viewport.

@Timer
Copy link
Member

Timer commented Nov 9, 2020

You must use position: 'relative':

<div style={{position: 'relative', width: '20px', height: '20px'}}>
  <Image src='image.src' layout='fill' />
</div>

@samjusaitis
Copy link

Would you expect this to go to the top-level wrapper or the <img /> component? Could you please explain the use-case and what you're trying to do?

I would expect it to go to the component similarly to all other props not referenced by next/image.

My use case is wanting to programatically determine if the image has loaded yet, and toggle a className based on that.

@nelsieborja
Copy link

+Refs via useRef hook.

@aulneau
Copy link

aulneau commented Nov 30, 2020

@Timer I would also love for this to forward refs, there are many reasons why this is important, as listed above. The ref should go to the underlying img tag in my mind.

Thanks!

@Xetera
Copy link

Xetera commented Dec 27, 2020

For extra context, the Image component exposes an onLoad event but adding onLoad does not reliably work for server side rendered images, the solution is checking img.complete on the ref instead. Not having a ref for the Image component prevents us from doing that kind of check for cases like rendering a skeleton while an image is loading.

@mattrossman
Copy link

Echoing @Xetera, a reliable way to detect the load state of an image is required in order to animate its entrance with react-spring, so I would expect access to the img element.

@Timer Timer modified the milestones: 10.x.x, backlog Jan 6, 2021
@TooTallNate
Copy link
Member

TooTallNate commented Jan 16, 2021

I need a reference to the <img> element in order to define an ontransitionend event handler.

Update: You can set the onTransitionEnd prop on the Image component, and it will be passed through to the img. This probably works for most cases.

I think this issue is still valid though because for my particular use case it is easier to assign the event handler manually (currently I'm storing a ref to the parent element and then doing a ref.querySelector() call).

@sigrundis
Copy link

+1 on wanting to pass ref down to to be able to access "imgRef.current.complete".

I want to use the next/image component for the Automatic Image Optimization, but I want to trigger an animation when the image is loaded.

Does anyone have a suggestion about workaround in the meantime?

@f0rr0
Copy link

f0rr0 commented Feb 15, 2021

How would one go about using any kind of animation library (say framer motion in my case) with the Image component if framer can't attach it's ref onto the container?

There should be affordance to add a user defined ref on the image as well as the container div.

Also currently I don't think this component is usable with framer even if ref is added because the layout prop collides with framer motion's layout prop.

@pffigueiredo
Copy link

pffigueiredo commented Feb 18, 2021

We are currently using 'react-content-loader' as our skeleton/animation lib, and this is how we handled not having access to the next image ref.

We are basically setting the image as imgLoaded, when it's not hidden anymore (by the visibility property in the element's style attribute). It was the best we could came up, at least in terms of syncing when the image was ready to be showed to the user and removing the loader.

const ExampleImage = (): ReactElement => {
  const [imgLoaded, setImgLoaded] = useState(false);

  const handleImageLoad = (event: React.SyntheticEvent<HTMLImageElement, Event>): void => {
    const target = event.target as HTMLImageElement;
    if (target.complete && target.style.visibility !== 'hidden') {
      setImgLoaded(true);
    }
  };

  return (
    <Box>
      {!imgLoaded && <Loader />}
      <Box height={imgHeight}>
        <Image src={imgSrc} onLoad={handleImageLoad} alt={imgAlt} layout="fill" />
      </Box>
    </Box>
  );
};

@Weffe
Copy link

Weffe commented Feb 24, 2021

I also support being able to forward a ref to the root image element. In my case I need it for a feature I am building out with framer-motion and need access to the image's complete property.

I've opened up PR #22482 that should address this issue.

@jrydberg
Copy link

One thing I'm looking into is being able to extract the dominant colors from an image. Would be useful to have a ref to the img for that.

@enterframe
Copy link

+1

@TommySorensen
Copy link

@pffigueiredo
There is also a different solution to knowing when the image is loaded. You can avoid looking for the 1x1 gif file so the onLoad event is only fired on the actual image.

const ExampleImage = () => {
  const [imgLoaded, setImgLoaded] = useState(false);

  return (
    <Image
      width={100}
      height={100}
      onLoad={event => {
        // next/image use an 1x1 px git as placeholder. We only want the onLoad event on the actual image
        if (event.target.src.indexOf('data:image/gif;base64') < 0) {
          setImgLoaded(true)
        }
      }}
    />
  );
};

@styfle
Copy link
Member

styfle commented Jun 29, 2021

Hi everyone, thanks for the feedback!

I read through all the comments here and it seems the majority use cases for ref could be solved by adding a new prop onLoadingComplete().

Does that sounds like a good solution? 👍 or 👎

@styfle styfle self-assigned this Jun 30, 2021
@styfle styfle removed this from the backlog milestone Jun 30, 2021
@styfle styfle added this to the Iteration 23 milestone Jun 30, 2021
@timneutkens
Copy link
Member

If replying with 👎 to @styfle's post please share the case that you're trying to solve by having access to ref so that it can be considered as well 👍

@kodiakhq kodiakhq bot closed this as completed in #26824 Jul 1, 2021
kodiakhq bot pushed a commit that referenced this issue Jul 1, 2021
This adds a new prop, `onLoadingComplete()`, to handle the most common use case of `ref`.

I also added docs and a warning when using `ref` so we recommend the new prop instead.
 
- Fixes #18398 
- Fixes #22482
@styfle
Copy link
Member

styfle commented Jul 1, 2021

We released onLoadingComplete() today in [email protected].

Try it out now with yarn add next@canary, thanks!

@yume-chan
Copy link

My use case: query image natural size, animate it using DOM API.

Other use cases already mentioned by others:

  1. Query element size.
  2. Use with other animation library that's not React specific or use DOM API for performance reason.
  3. Draw it onto canvas.

leimonio added a commit to leimonio/next.js that referenced this issue Jul 6, 2021
* [WIP] Add cms notion integration example - index page

* Add cover images for pages

* Add post page

* Remove preview functionality

* Add module.exports to security headers documentation (vercel#26466)

Without `module.exports`, the provided code won't work if just pasted into `next.config.js`

## Documentation / Examples

- [x] Make sure the linting passes

* fix: ignore invalid accept-language header (vercel#26476)



Fixes vercel#22329

## Bug

- [x] Related issues linked using fixes vercel#22329
- [x] Integration tests added

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.

## Documentation / Examples

- [ ] Make sure the linting passes

* chore: Add Alex to lint documentation (vercel#26354)

This Pull Request adds [Alex](https://alexjs.com/) to our documentation. It catches insensitive, inconsiderate writing. 

The original PR (vercel#25821) is too large so I have decided to break it down into smaller PRs. This PR is the first part. Then I will continue to add the rest of the documentation in smaller PRs.

## More Information on Alex:
https://alexjs.com/
https://github.com/get-alex/alex

## Documentation / Examples

- [x] Make sure the linting passes

* Fix domain locales not available on client (vercel#26083)

* bug: Fix domain locales not available on client

* Add test case

* update tests

Co-authored-by: JJ Kasper <[email protected]>

* Update to only add image import types when enabled (vercel#26485)

* Update to only add image import types when enabled

* add type check to test

* Update i18n fields in docs (vercel#26492)



Closes: vercel#24991

## Documentation / Examples

- [x] Make sure the linting passes

* v11.0.1-canary.7

* Strongly type `Router.events.on` and `Router.events.off` (vercel#26456)

This strongly types `Router.events.on` and `Router.events.off`. Previously the event type was `string` but now it's `'routeChangeStart' | 'beforeHistoryChange' | 'routeChangeComplete' | 'routeChangeError' | 'hashChangeStart' | 'hashChangeComplete'`


## Bug

- ~[ ] Related issues linked using `fixes #number`~
- [x] Integration tests added

Closes vercel#25679
Closes vercel#23753
Closes vercel#15497

* Update next Link & Image components

* Ensure image-types file is included (vercel#26495)

* Update react & react-dom to v17

* Update tailwind to use jit

* v11.0.1-canary.8

* v11.0.1

* Don't test image domains in test env (vercel#26502)

fixes vercel#21549

Co-authored-by: JJ Kasper <[email protected]>

* docs: updated minimum Node.js version (vercel#26528)

## Documentation / Examples

- [x] Make sure the linting passes


According to new requirements in `package.json` minimum Node.js version for now is 12.0

* Update next-env note in docs (vercel#26536)

This ensures we don't recommend editing the `next-env` file since we need to be able to tweak it to accept future types we add in Next.js


## Documentation / Examples

- [x] Make sure the linting passes

Closes: vercel#26533

* [examples] Fix ssr-caching example. (vercel#26540)

Closes vercel#12019 with a better example of proper SSR caching.

* Fix props not updating when changing the locale and keeping hash (vercel#26205)



Currently, there is only a `hashChangeStart` and subsequent `hashChangeComplete` event and no props update (which would be used to get translations, etc.).
Happy for any feedback

fixes vercel#23467

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.

## Documentation / Examples

- [ ] Make sure the linting passes

* Add note about adding additional types (vercel#26545)

* Add note about adding additional types

* Update to bold edit also

* Apply suggestions from code review

Co-authored-by: Steven <[email protected]>

* Allow user to override next-image-loader (vercel#26548)

In PR vercel#26281, we solved one use case but broke another.

This PR will allow the user to [override the built-in loader](vercel#26281 (comment)) via custom webpack config.

* v11.0.2-canary.0

* chore: Enable Alex documentation linting for error pages (vercel#26526)

* Update SWR example to include fetcher function. (vercel#26520)

* Previous example doesn't work

* Apply suggestions from code review

* Update docs/basic-features/data-fetching.md

* lint-fix

Co-authored-by: Lee Robinson <[email protected]>
Co-authored-by: JJ Kasper <[email protected]>

* tailwind examps upgraded to v2.2 (vercel#26549)



## Documentation / Examples

- [x] Make sure the linting passes

* doc: prettify docs for next script (vercel#26572)



x-ref: vercel#26518 (comment)

## Documentation / Examples

- [x] Make sure the linting passes

* Add logging when a custom babelrc is loaded (vercel#26570)

Partially solves vercel#26539 by adding back the log output when a config file is used



## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.

## Documentation / Examples

- [ ] Make sure the linting passes

* Add comment to not edit in next-env file (vercel#26573)

This adds a comment to the generated `next-env.d.ts` to mention it should not be edited pointing to the documentation which contains an example of adding custom types separately. 

x-ref: vercel#26560

## Documentation / Examples

- [x] Make sure the linting passes

* Separate node polyfill test from basic suite (vercel#26550)

* Separate node polyfill test from basic suite

* update test

* fix with-loading example for next 11 (vercel#26569)

## Documentation / Examples

- [X] Make sure the linting passes

This PR updates the with-loading example to follow the documentation of router events for next 11

* v11.0.2-canary.1

* Add trace url on bootup (vercel#26594)

* Add trace url on bootup

* Update whitelist -> accesslist

* Add name to webpack-invalidated

* v11.0.2-canary.2

* Add check for ObjectExpression when iterating on <link> tags for font optimization (vercel#26608)

Fixes vercel#26547



## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.

## Documentation / Examples

- [ ] Make sure the linting passes

* Enable Alex documentation linting for docs (vercel#26598)

* Add link to live demo already hosted (vercel#25718)

* Add link to live demo already hosted

To make it easier for people to simply see the live example without having to deploy a whole new project

* update link


Co-authored-by: JJ Kasper <[email protected]>

* Update next/image docs for relative parent with layout=fill. (vercel#26615)

vercel#18739 (reply in thread)

* Fix GSP redirect cache error (vercel#26627)

This makes sure we don't attempt flushing cache info to disk for `getStaticProps` `redirect` items with `revalidate`

Fixes: vercel#20816

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added

* Correct statusCode when visiting _error directly (vercel#26610)

This fixes non-stop reloading when visiting `_error` directly in development caused by the `statusCode` being 200 unexpectedly while HMR returns the page as `invalid` which triggers `on-demand-entries` to reload the page. 

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added

Fixes: vercel#8036
x-ref: vercel#8033

* fix: next dynamic with jest (vercel#26614)



Fixes vercel#19862

Avoid executing `webpack` property on `loadableGenerated` of loadable component compiled from `next/dynamic` when `require.resolveWeak` is unavailable due to jest runtime missing `require.resolveWeak`.

## Bug

- [x] Related issues linked using `fixes #number`
- [x] unit tests added

* Ensure API routes are not available under the locale (vercel#26629)

This ensures API routes are not available under the locale path since API routes don't need to be localized and we don't provide the locale to the API in any way currently so the user wouldn't be aware if the localized API route was visited instead of the non-localized. 

Fixes: vercel#25790

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added

* v11.0.2-canary.3

* Fix image content type octet stream 400 (vercel#26705)

Fixes vercel#23523 by adding image content type detection

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added

* Update layouts example to persist state across client-side transitions. (vercel#26706)

* Update layouts example

* Update examples/layout-component/components/layout.js

Co-authored-by: JJ Kasper <[email protected]>

* Fix typo on "occured" to "occurred" (vercel#26709)

* fix: typo occured -> occurred

* fix: typo occured -> occurred

* fix: typo occured -> occurred

* fix: typo occured -> occurred

* lint-fix


Co-authored-by: JJ Kasper <[email protected]>

* [ESLint] Adds --max-warnings flag to `next lint` (vercel#26697)

Adds `--max-warnings` flag to `next lint` to enable setting of a maximum warning threshold.

Fixes vercel#26671

* update with-redux-toolkit-typescript (vercel#26714)



## Bug

- [X] Related issues linked using `fixes vercel#26713 `

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.

## Documentation / Examples

- [X] Make sure the linting passes

## Screenshots

After this small change the warning disappears.
![image](https://user-images.githubusercontent.com/47717492/123749377-fd56fb80-d8d2-11eb-8b70-dbb7f6f16050.png)

* Simplify `next-dev-server` implementation (vercel#26230)

`next-dev-server` having its own implementations of `renderToHTML` and `renderErrorToHTML` has historically made reasoning about streaming hard, as it adds additional places where status codes are explicitly set and the full HTML is blocked on.

Instead, this PR simplifies things considerably by moving the majority of the custom logic for e.g. hot reloading and on-demand compilation to when we're resolving the page to be loaded, rather than upfront when handling the request. It also cleans up a few other details (e.g. default error page rendering) that managed to creep into the base implementation over time.

One unfortunate side effect is that this makes compilation errors slightly more difficult. Previously, we'd render them directly. Now, we have to rethrow them. But since they've already been logged (by the watch pipeline), we have to make sure they don't get logged again.

* Update PR labeler action

* Simplify stats action (vercel#26751)

* Move code shared between server/client to "shared" folder (vercel#26734)

* Move next-server directory files to server directory (vercel#26756)

* Move next-server directory files to server directory

* Update tests

* Update paths in other places

* Support new hydrate API in latest react 18 alpha release (vercel#26664)

* fix: react 18 new hydration API

* support react 18

* compat latest react only, fix resolved version

* fix tests

* Some changes based on reactwg/react-18#5

* fix test

Co-authored-by: Tim Neutkens <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* Disable build-output size specific tests (vercel#26769)

* Disable build-output size specific tests

* remove size-limit test

* lint-fix

* Add upstream `max-age` to optimized image (vercel#26739)

This solves the main use case from Issue vercel#19914.

Previously, we would set the `Cache-Control` header to a constant and rely on the server cache. This would mean the browser would always request the image and the server could response with 304 Not Modified to omit the response body.

This PR changes the behavior such that the `max-age` will propagate from the upstream server to the Next.js Image Optimization Server and allow browser caching. ("upstream" meaning external server or just an internal route to an image)

This PR does not change the `max-age` for static imports which will remain `public, max-age=315360000, immutable`.

#### Pros:
- Fewer HTTP requests after initial browser visit
- User configurable `max-age` via the upstream image `Cache-Control` header

#### Cons:
- ~~Might be annoying for `next dev` when modifying a source image~~ (solved: use `max-age=0` for dev)
- Might cause browser to cache longer than expected (up to 2x longer than the server cache if requested in the last second before expiration)

## Bug

- [x] Related issues linked using `fixes #number`

* Fix blurred image position when using objectPosition (vercel#26590)



## Bug

fixes vercel#26309

## Documentation / Examples

see vercel#26309

- [ ] Make sure the linting passes

* Update azure tests (vercel#26779)

* Stabilize relay-analytics test (vercel#26782)

* Leverage blocked page for _error (vercel#26748)



## Enhance

simplify detection for visiting `_error`

x-ref: vercel#26610

* Update codeowners to add new maintainers (vercel#26770)

* examples: fix typo `lunix` → `linux` (vercel#26796)



## Bug

- [ ] ~~Related issues linked using `fixes #number`~~
- [ ] ~~Integration tests added~~

## Feature

- [ ] ~~Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.~~
- [ ] ~~Related issues linked using `fixes #number`~~
- [ ] ~~Integration tests added~~
- [ ] ~~Documentation added~~
- [ ] ~~Telemetry added. In case of a feature if it's used or not.~~

## Documentation / Examples

- [x] Make sure the linting passes

* Update repo scripts to separate folder (vercel#26787)

* fix: detect loop in client error page (vercel#26567)

Co-authored-by: Tobias Koppers <[email protected]>
Co-authored-by: Tim Neutkens <[email protected]>

* Update snapshot for font-optimization test (vercel#26823)

This fixes the `font-optimization` test failing from the links/content changing slightly in the snapshot. 

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.

## Documentation / Examples

- [ ] Make sure the linting passes

* Add `onLoadingComplete()` prop to Image component (vercel#26824)

This adds a new prop, `onLoadingComplete()`, to handle the most common use case of `ref`.

I also added docs and a warning when using `ref` so we recommend the new prop instead.
 
- Fixes vercel#18398 
- Fixes vercel#22482

* Add "Vary: Accept" header to /_next/image responses (vercel#26788)

This pull request adds "Vary: Accept" header to responses from the image optimizer (i.e. the /_next/image endpoint).

The image optimizer prefers re-encoding JPG files to WebP, but some browsers (such as Safari 14 on Catalina) do not yet support WebP. In such cases the optimizer uses the Accept header sent by the browser to send out a JPG response. Thus the optimizer's response may depend on the Accept header.

Potential caching proxies can be informed of this fact by adding "Vary: Accept" to the response headers. Otherwise WebP data may be served to browsers that do not support it, for example in the following scenario:
 * A browser that supports WebP requests the JPG. The optimizer re-encodes it to WebP. The proxy caches the WebP data.
 * After this another browser that doesn't support WebP requests the JPG. The proxy sends the WebP data to the browser.

- [x] Integration tests added
- [x] Make sure the linting passes

* Fix using-preact example deps (vercel#26821)

Fix after vercel#26133

* Add additional tests for image type detection (vercel#26832)

Adding additional tests. Follow up to vercel#26705

* Fix immutable header for image with static import & unoptimized (vercel#26836)

Fixes vercel#26587

* Update `publish-canary` script to include checkout (vercel#26840)

* Update `publish-canary` script to include checkout

* Update contrib with publishing section

* v11.0.2-canary.4

* Make sure 404 pages do not get cached by a CDN when using next start (vercel#24983)

Co-authored-by: Jiachi Liu <[email protected]>
Co-authored-by: JJ Kasper <[email protected]>

* Update to environment-variable.md (vercel#26863)



## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [x] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.

## Documentation / Examples

- [ ] Make sure the linting passes

## Why the change
I lost quite a few hours figuring out why my environment variable was `undefined` in the browser.
I read about the built-in support and was like: "Oh nice" and didn't read much further. I missed the part about how you need to prefix your variables with `NEXT_PUBLIC_` in order to expose them to the browser.
I think a hint to this in the anchor link to that section will make it more discoverable as it's then mentioned near the top and will save people who are like me some time and a headache.

* Don't emit duplicate image files (vercel#26843)

fixes vercel#26607

This change makes it so the image loader plugin only emits a file while processing an image import for the client. The final generated image URL was already the same in SSR and CSR anyway, so this change doesn't have any functional impact.

I also changed the name of the static page in the image component tests, since it was causing some conflicts with the static assets folder.

* Add instructions on how to add nextjs.org/docs/messages urls (vercel#26865)

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* Rename example folder to with-notion

* Update references of example

* Reorder docs manifest and rename to Script Optimization. (vercel#26874)

So there's _conformance_ between the other docs / optimizations.

* Fix typo on "occured" to "occurred" (vercel#26876)

- Fix typo on "occured" to "occurred"

* Update README

Co-authored-by: Sam Robbins <[email protected]>
Co-authored-by: Jiachi Liu <[email protected]>
Co-authored-by: Peter Mekhaeil <[email protected]>
Co-authored-by: Rob Vermeer <[email protected]>
Co-authored-by: JJ Kasper <[email protected]>
Co-authored-by: Brandon Bayer <[email protected]>
Co-authored-by: Alex Castle <[email protected]>
Co-authored-by: Vitaly Baev <[email protected]>
Co-authored-by: Lee Robinson <[email protected]>
Co-authored-by: destruc7i0n <[email protected]>
Co-authored-by: Steven <[email protected]>
Co-authored-by: Joshua Byrd <[email protected]>
Co-authored-by: Pranav P <[email protected]>
Co-authored-by: Tim Neutkens <[email protected]>
Co-authored-by: John <[email protected]>
Co-authored-by: Tim Neutkens <[email protected]>
Co-authored-by: Vadorequest <[email protected]>
Co-authored-by: hiro <[email protected]>
Co-authored-by: Houssein Djirdeh <[email protected]>
Co-authored-by: Soham Shah <[email protected]>
Co-authored-by: Gerald Monaco <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Co-authored-by: Nils Schönwald <[email protected]>
Co-authored-by: D. Kasi Pavan Kumar <[email protected]>
Co-authored-by: Tobias Koppers <[email protected]>
Co-authored-by: Joachim Viide <[email protected]>
Co-authored-by: Artur Sedlukha <[email protected]>
Co-authored-by: Paul van den Dool <[email protected]>
Co-authored-by: hiro <[email protected]>
@Kerosz
Copy link

Kerosz commented Jul 29, 2021

I would also prefer the ref approach, my use case is doing animations that will require me to have access to the image element. I'm not sure what the implications of adding the option to forwardRef's would be, as I've not looked into how the Image component works exactly, but it would be nice to have that option enabled for whoever wants to use it.

@styfle
Copy link
Member

styfle commented Jul 31, 2021

We can handle the "query image natural size" use case by returning those values in onLoadingComplete().

See #27213 (comment) for my suggestion.

Update: See the docs https://nextjs.org/docs/api-reference/next/image#onloadingcomplete

flybayer pushed a commit to blitz-js/next.js that referenced this issue Aug 19, 2021
This adds a new prop, `onLoadingComplete()`, to handle the most common use case of `ref`.

I also added docs and a warning when using `ref` so we recommend the new prop instead.
 
- Fixes vercel#18398 
- Fixes vercel#22482
@harisraharjo
Copy link

harisraharjo commented Sep 9, 2021

If replying with 👎 to @styfle's post please share the case that you're trying to solve by having access to ref so that it can be considered as well 👍

How about when trying to use tensorflow mobilenet?
The scenario is this:

  1. User upload their image and create the URL
  2. preview the image with next/image or img tag. In Here we put the URL as the src and reference this tag's ref to a useRef variable
  3. execute model.classify method from tensorflow with the useRef variable value

The scenario above won't work with next/image because next/image doesn't forward it's ref

Here is the model.classify method snippet. Source: https://github.com/tensorflow/tfjs-models/tree/master/mobilenet
model.classify( img: tf.Tensor3D | ImageData | HTMLImageElement | HTMLCanvasElement | HTMLVideoElement, topk?: number )

@dbismut
Copy link

dbismut commented Oct 10, 2021

I want to animate the Image component when it enters the viewport. At this point, the only solution seems to be wrapping the Image component inside a div that I can pass a ref to (which btw triggers a warning in dev mode about its position being static).

@dbismut
Copy link

dbismut commented Oct 30, 2021

Is there any updates on this? As it's been said before, triggering animation based on when the image shows on screen requires getting a ref of some sort, except if I'm missing something.

Is there a problem with passing the ref to the img tag or the wrapping div? Or remove the warning about static positioning?

@luigi-derson
Copy link

In my case I would like to remove the inline styles you add to the image element when using placeholder="blur"

img.style.filter = 'none'

I'm adding styles to the image that are being overridden by those inline styles.

@styfle
Copy link
Member

styfle commented Dec 18, 2021

@luigi-derson This looks like a bug, no need for ref.

This will be fixed in PR #32623, thanks!

@luigi-derson
Copy link

@luigi-derson This looks like a bug, no need for ref.

This will be fixed in PR #32623, thanks!

It's all right! Isn't it the same for the next two lines? backgroundSize and backgroundImage?

kodiakhq bot pushed a commit that referenced this issue Dec 20, 2021
This PR is a follow up to PR #32623 to fix the remaining blur styles.

These are unlikely to be overridden by the user but this PR is necessary to make `placeholder=empty` (default behavior) and `placeholder=blur` end up with the same inline styles on the loaded image.

Related to #18398 (comment)
@maciekgrzybek
Copy link

Are there any updates on this? I have a use case where I need to get the image data and put it on the canvas - so I need a ref for that. Would be awesome to have this :)

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 27, 2022
natew pushed a commit to natew/next.js that referenced this issue Feb 16, 2022
ijjk pushed a commit that referenced this issue Sep 8, 2022
jimCresswell pushed a commit to jimCresswell/next.js that referenced this issue Nov 25, 2022
Add ref forwarding for next/image with integration test


- fixes vercel#42885
- fixes vercel#18398 (this one was closed with `onLoadingComplete`)

## Feature

- [x] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [x] Documentation added


Co-authored-by: Steven <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.