color-js / color.js

Color conversion & manipulation library by the editors of the CSS Color specifications
https://colorjs.io
MIT License
1.87k stars 82 forks source link

.display and .toString different behavior on nextjs in dev and build mode #260

Closed star-over closed 6 months ago

star-over commented 1 year ago

Trying to use color.display({"format": "hex"}) as well as .toString({"format": "hex"}) in nextjs v13. In dev it works fine, outputs #111 for example, but in prod mode it outputs color(srgb 0.0667 0.0667 0.0667) when running bundle after next build. So I use this method to get the same result in dev and in prod mode Color.defaults.display_space.formats.hex.serialize(color.coords). Is there any "correct" solution to output hexadecimal representation of colour (#123456)?

LeaVerou commented 1 year ago

This sounds very similar to an issue we had recently and fixed in https://github.com/LeaVerou/color.js/pull/239 Could you please make sure you’re using v0.4.2 of Color.js?

star-over commented 1 year ago

Pretty shure. "colorjs.io": "^0.4.2" - record in package.json

nick-inkeep commented 1 year ago

@LeaVerou any update on this? Facing same issue.

LeaVerou commented 1 year ago

Hey, sorry this slipped through the cracks. Could you make a reduced testcase that I could look at and play with (e.g. on stackblitz or something)?

sarah-inkeep commented 1 year ago

Hi @LeaVerou! I made a reduced test case here. When it's running in dev mode (next dev) you'll see the color is in the expect hex format, but if you run next build && next start and then refresh, it outputs as color(srgb 0.3216 0.1843 0.7882).

jh3y commented 1 year ago

Can confirm, having this issue too, albeit slightly differently.

let color = new Color('hsl(0 100% 50%)')
color.to('p3').toString() // Dev = color(display-p3 0.9175 0.2003 0.1386)
color.to('p3').toString() // Prod = color(p3 0.9175 0.2003 0.1386)

Works fine for other formats I've found 🤔

lloydk commented 6 months ago

I took a look at this, the reduced test case was super helpful.

When the test app is run in dev mode the code for the page is run in the browser and works correctly.

In production mode (next build && next start) Next.js generates static pages and the code for the page is run as part of the build process and does not run correctly.

The color.js code that is failing is the assignment of the format variable: https://github.com/color-js/color.js/blob/b896c0d20f96be8f117488d2935c7a1443e66845/src/serialize.js#L26-L28

If you print out the value of the format variable (pay attention to the location of the console.log statement) the value of format is "hex" which is the value that was passed into the serialize function.

    format = color.space.getFormat(format)
           ?? color.space.getFormat("default")
           ?? ColorSpace.DEFAULT_FORMAT;

    inGamut ||= format.toGamut;

    let coords = color.coords.slice(); // clone so we can manipulate it

    if (inGamut && !checkInGamut(color)) {
        // FIXME what happens if the color contains NaNs?
        coords = toGamut(clone(color), inGamut === true ? undefined : inGamut).coords;
    }

    console.log(`format is ${format}`);

If you move the console.log statement just below the assignment of the format variable the code runs correctly.

    format = color.space.getFormat(format)
           ?? color.space.getFormat("default")
           ?? ColorSpace.DEFAULT_FORMAT;
    console.log(`format is ${format}`);

    inGamut ||= format.toGamut;

If you access a property of format right after the assignment the code runs correctly.

    format = color.space.getFormat(format)
           ?? color.space.getFormat("default")
           ?? ColorSpace.DEFAULT_FORMAT;

    let x = format.type;

If you replace the nullish coalescing operators with if/else statements the code runs correctly.

    format = color.space.getFormat(format);

    if (!format) {
        format = color.space.getFormat("default");
    }
    else {
        if (!format) {
            format = ColorSpace.DEFAULT_FORMAT;
        }
    }

This also works and maybe should be the preferred workaround?

    format = color.space.getFormat(format) ?? color.space.getFormat("default");

    if (!format) {
        format = ColorSpace.DEFAULT_FORMAT;
    }

TLDR: The issue appears to be Next.js bug but there are some ways to work around the issue.

LeaVerou commented 6 months ago

Thank you so much for tracking it down @lloydk! Should we close this issue then?

lloydk commented 6 months ago

Thank you so much for tracking it down @lloydk! Should we close this issue then?

Up to you, we can either close the issue or I can create a PR for one of the work arounds.

svgeesus commented 6 months ago

Maybe we should have a FAQ listing these sorts of known issues and workarounds?

lloydk commented 6 months ago

Changing the code from

    inGamut ||= format.toGamut;

    let coords = color.coords.slice(); // clone so we can manipulate it

to

    let coords = color.coords.slice(); // clone so we can manipulate it

    inGamut ||= format.toGamut;

seems to work for me so I'll create a PR for that change.