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 optimization fails due to version mismatch #406

Closed
Seanw265 opened this issue Apr 24, 2024 · 15 comments · Fixed by #409
Closed

Image optimization fails due to version mismatch #406

Seanw265 opened this issue Apr 24, 2024 · 15 comments · Fixed by #409

Comments

@Seanw265
Copy link

Seanw265 commented Apr 24, 2024

I'm using OpenNext (v2.3.7) through the SST NextJsSite construct.

I recently had an issue where images loaded through the <Image> tag failed at the optimization step. After digging into the issue, I learned that Next changed the optimizer call signature in v14.1.1, hence #374 and #377. I was running v14.2.2.

Digging in further, I learned that the OpenNext image optimization plugin was not applied during my build step. This is because the plugin is gated behind a version check for v14.1.1. BUT when a Next version isn't specified in the initial options, OpenNext attempts to extract it from package.json.

However, the version specified in package.json is not necessarily the version that's installed. In my case, I had "next": "^14.0.4" in my package.json, but the actual installed version was v14.2.2. This resulted in a logic mismatch between versions, and the image optimizer function simply returned Cannot read properties of undefined (reading '0') every time it was invoked.

It seems like an appropriate solution would be to instead obtain the installed version number from package-lock.json or the module itself, but I defer to the original authors.

ETA: I was able to fix my original issue by bumping the next version in package.json to ^14.2.2.

@conico974
Copy link
Collaborator

I think the current solution is the best we can do without overcomplicating things, and to me having two different versions in package.json and package-lock.json is more an issue ( Maybe i'm missing something here )

We cannot just use package-lock.json, if we'd do that we need to support pnpm-lock.yaml and yarn.lock and the bun lockfile ( which is a binary files ). To add even more complexity to that in a monorepo settings, you could have multiple next app with different versions, all in a single lockfile.

The other solutions could be to use require("next/package.json").version, but this won't work for cases where you're running OpenNext from a different directory than next ( which is possible )

@khuezy
Copy link
Collaborator

khuezy commented Apr 25, 2024

@Seanw265 thanks for the investigation, you have a good point... but to add to what @conico974 said: nextjs has been known to have breaking changes even in their .patch releases. They don't adhere semversioning, so I'd recommend fixing your next version instead of using ^.

To be safe, you should use ~ instead of ^, but in general, especially with node, you should fix your packages b/c broken minor/patches in node happens more than you think.

@Seanw265
Copy link
Author

Seanw265 commented Apr 25, 2024

@conico974 @khuezy thanks for your response.

To be safe, you should use ~ instead of ^, but in general, especially with node, you should fix your packages b/c broken minor/patches in node happens more than you think.

Please correct me if I'm wrong, but I believe this issue would still have occurred even if I had used ~ in package.json if it contained "next":"~14.1.0". That aside, using ^ and ~ is not out of the ordinary. Vercel itself uses it in their monorepo example which is provided as a template.

But I can certainly understand where you're coming from. Without delving in too deep, our deployment process is set up in such a way that we would catch a breaking change before it's released, but I think we're the exception and not the rule.


As more projects adopt OpenNext and as Vercel releases new breaking changes to Nextjs that need to be addressed with plugins, this could become a larger issue.

Perhaps a low-effort solution would be to add something to the documentation that specifies that the next package should be pinned due to a reliance on the version number specified in package.json; failure to do so could result in unexpected behavior, as demonstrated in this ticket.

Going a step further, OpenNext could log a warning during the build step if the app's next version is not pinned. This should be simpler than identifying the installed version of next, while being more robust than a simple documentation change.

@khuezy
Copy link
Collaborator

khuezy commented Apr 25, 2024

Yea, that wouldn't have prevented the issue, but it'll minimize packages that have breaking changes in the minor version. We should look into this to improve actual version detection.

@kelvinCJJ
Copy link

@Seanw265 I tried to update it to nextjs 14.1. but still getting "Cannot read properties of undefined (reading '0')"

@conico974
Copy link
Collaborator

@kelvinCJJ Are you using windows ?

@kelvinCJJ
Copy link

Yes I am using windows and downloaded WSL, but it still doesnt seem to work

@conico974
Copy link
Collaborator

@kelvinCJJ What version of next is inside package.json ? ( With ^ or ~ if they're here )
And you run open-next build from within WSL ? Using linux filesystem or windows ?

@kelvinCJJ
Copy link

Not sure of the actual issue, but downgrading to next@14.1.0 solved it for me

@conico974
Copy link
Collaborator

@kelvinCJJ That's not really a fix, if you see this error it very likely means that the plugin system used internally don't work.
If that's the case, it's very likely that a lot of things might break in production ( ISR, On demand revalidation, Middleware etc... )
#385 (comment)

@kelvinCJJ
Copy link

I am using nextjs ^14.1.0 on windows. I have installed WSL and currently only use sst dev and sst deploy in my cli. How do I run open-next build from within WSL?

@conico974
Copy link
Collaborator

conico974 commented May 5, 2024

@kelvinCJJ I realize i forgot to ask you this, but which version of OpenNext are you using ?

If you didn't change the buildCommand in the sst constructs open-next build already runs on WSL.

But you missed one very important point here, you should pin your version of next in package.json ( i.e. no ^ and no ~ and run install again )
With what you have if you remove your lockfile and install again it will install next@14.2.3
Also as stated in the comment i linked, you very likely want to move your repo to the WSL filesystem and work only there ( or use some CI to deploy like github actions )

And if you want to check if the plugin system is running fine, run OPEN_NEXT_DEBUG=true npx open-next@2.3.9 build inside WSL and post the result here

@kelvinCJJ
Copy link

kelvinCJJ commented May 5, 2024

I am using 2.3.9.
I did not change the buildCommand in the nextjs construct.
I will take note of the next version

I already have wsl installed and deployed as per normal without any changes in the build command. This should mean I am running it in WSL yeah?

Update:
DEBUG Applying plugins:: [opennext-13.5-serverHandler,opennext-13.5-util,opennext-13.5-default] for Next version: 14.1.0

@conico974
Copy link
Collaborator

@kelvinCJJ If that's the only output you get, then it's broken. This is what you're supposed to have

DEBUG Applying plugins:: [opennext-13.5-serverHandler,opennext-13.5-util,opennext-13.5-default] for Next version: 14.1.0
DEBUG Open-next plugin opennext-13.5-serverHandler -- Applying override for imports from ./13.5/serverHandler.js
DEBUG Open-next plugin opennext-13.5-serverHandler -- Applying override for handler from ./13.5/serverHandler.js
DEBUG Open-next plugin opennext-13.5-default -- Applying override for imports from ./default.replacement.js
DEBUG Open-next plugin opennext-13.5-default -- Applying override for processInternalEvent from ./default.replacement.js
DEBUG Open-next plugin opennext-13.5-default -- Applying override for postProcessResponse from ./default.replacement.js
DEBUG Open-next plugin opennext-13.5-util -- Applying override for requestHandler from ./14.1/util.js
DEBUG Open-next plugin opennext-13.5-util -- Applying override for requireHooks from ./14.1/util.js

My advice, just use github actions

@Seanw265
Copy link
Author

Seanw265 commented May 7, 2024

@kelvinCJJ and anyone else who experiences this issue:

I'm simplifying things a bit, but this should cover most cases. You have two options.

Option 1

You may "pin" the next package to a specific version. This means that your package.json entry for next should NOT contain ~ or ^.

For example:
"next":"14.1.0"
"next":"^14.1.0" ❌ no good
"next":"~14.1.0" ❌ no good

Option 2

If you must continue to use ~ or ^ in your package.json file, you must set the numeric portion of your version to something >= 14.1.1.

For example:
"next":"~14.1.1"
"next":"^14.1.1"
"next":"^14.2.0"
"next":"~14.1.0" ❌ no good
"next":"^14.0.4" ❌ no good


Afterwards

After adjusting your package.json file, you should consider performing a clean install of all of your dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants