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

Declare 'extend' variable in docker.js #784

Merged
merged 2 commits into from
Jan 10, 2025
Merged

Conversation

gaberudy
Copy link
Contributor

@gaberudy gaberudy commented Jan 6, 2025

This 'extend' variable is not being defined which is causing errors in node v20.

This 'extend' variable is not being defined which is causing errors in node v20.
@apocas
Copy link
Owner

apocas commented Jan 6, 2025

https://github.com/apocas/dockerode/actions/runs/12610511590/job/35145175728

Not seeing any issues in Node 20.x tests, could you detail?

@gaberudy
Copy link
Contributor Author

gaberudy commented Jan 8, 2025

I'm not sure of why this isn't causing issues in the tests, but new to v4.0.3 our esbuild bundle of our server with dockerode won't start because of this missing "var" declaration.

I wonder if it was in a "," chain and then moved to it's own statement, but it's clearly an undefined variable. I am getting errors like:

/opt/backend/auth_server.cjs:135193
    extend = util2.extend;
           ^
ReferenceError: extend is not defined
    at ../../node_modules/dockerode/lib/docker.js (/opt/backend/auth_server.cjs:135193:12)

@apocas
Copy link
Owner

apocas commented Jan 8, 2025

Yeah it's a typo, I missed the ";" before.
Just convert it into a ",", no need to segregate it.

Yeah this will not trigger everywhere.

Updated fix to use ','
@gaberudy
Copy link
Contributor Author

@apocas Will you accept this PR now (replace ';' with ',')?

@apocas apocas merged commit 713a097 into apocas:master Jan 10, 2025
5 checks passed
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.

2 participants