Skip to content

feat: move "ipx" into optional dependencies which can be skipped - #380

Closed
PatrickHeneise wants to merge 2 commits into
nuxt:mainfrom
zentered:379-fix-ipx-dependency
Closed

feat: move "ipx" into optional dependencies which can be skipped#380
PatrickHeneise wants to merge 2 commits into
nuxt:mainfrom
zentered:379-fix-ipx-dependency

Conversation

@PatrickHeneise

Copy link
Copy Markdown
Contributor

Move ipx into optionalDependencies. These can be skipped with --no-optional (npm) or --ignore-optional (yarn) when external image optimization APIs are used (in production systems). sharp has a few install requirements and binaries which can be tricky in some situations.

Closes #379

Comment thread src/ipx.ts Outdated
@PatrickHeneise
PatrickHeneise requested a review from pi0 August 12, 2021 11:17
@PatrickHeneise

Copy link
Copy Markdown
Contributor Author

@pi0 would appreciate a review on this. Running into build issues again on M1.

@pi0

pi0 commented Sep 5, 2021

Copy link
Copy Markdown
Member

Hi @PatrickHeneise sorry for the delay and thanks again for helping on this. It is now merged via #403 and will be released shortly as a fix :)

BTW some notes:

  • Please when making a pull-request, ensure "Allow edit from maintainers" so that we can directly push cleanup before merge
  • It is usually better to avoid doing unrelated changes in a PR (in this case it was recreation of yarn.lock)
  • If ipx is failing on M1, your Node.js installation is probably old. Which I would recommend check docs to troubleshoot this issue. Even if not using ipx/sharp, it can probably be good to fix root cause on your local machine :)

@pi0 pi0 closed this Sep 5, 2021
@PatrickHeneise

PatrickHeneise commented Sep 6, 2021

Copy link
Copy Markdown
Contributor Author

Thanks @pi0. I didn't know about the "allow editing for maintainers" option, will make sure that's enabled next time. I'm running Node 16. The problems are cross-arch build issues when Docker is involved.

I'm not familiar with yarn, but when I move/remove something in the dependencies (which happened here), the lock file needs to be updated, as otherwise it's still installed on production systems with npm ci. Is that not the case with yarn?

@pi0

pi0 commented Sep 6, 2021

Copy link
Copy Markdown
Member

Indeed when updating dependencies, yarn lock needs to be updated :)

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 this pull request may close these issues.

Skipping sharp dependency?

2 participants