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

[JS] "gulp bundle:webpack" fails with Node.js 18 #36025

Closed
kou opened this issue Jun 10, 2023 · 12 comments · Fixed by #36089
Closed

[JS] "gulp bundle:webpack" fails with Node.js 18 #36025

kou opened this issue Jun 10, 2023 · 12 comments · Fixed by #36089

Comments

@kou
Copy link
Member

kou commented Jun 10, 2023

Describe the enhancement requested

#33504 reported the same problem but closed with a workaround (using Node.js 16 instead of 18).

$ cd js
$ npm install
$ node node_modules/gulp-cli/bin/gulp.js bundle:webpack
[06:33:00] Using gulpfile ~/work/cpp/arrow.kou/js/gulpfile.js
[06:33:00] Starting 'bundle:webpack'...
[06:33:00] 'bundle:webpack' errored after 246 ms
[06:33:00] Error [ERR_STREAM_PREMATURE_CLOSE]: Premature close
    at new NodeError (node:internal/errors:400:5)
    at Stream.<anonymous> (node:internal/streams/pipeline:352:14)
    at Stream.emit (node:events:525:35)
    at Stream.emit (node:domain:552:15)
    at stream.destroy (node_modules/through/index.js:84:12)
    at _end (node_modules/through/index.js:67:14)
    at stream.end (node_modules/through/index.js:74:5)
    at DestroyableTransform.onend (node_modules/readable-stream/lib/_stream_readable.js:577:10)
    at Object.onceWrapper (node:events:627:28)
    at DestroyableTransform.emit (node:events:525:35)

Component(s)

JavaScript

@kou
Copy link
Member Author

kou commented Jun 10, 2023

@abetomo Can you fix this?

@abetomo
Copy link
Contributor

abetomo commented Jun 11, 2023

I will check it and fix it.

@abetomo
Copy link
Contributor

abetomo commented Jun 11, 2023

take

@abetomo
Copy link
Contributor

abetomo commented Jun 11, 2023

I ran the same command as reported and got the following error.

$ cd js
$ npm install
$ node node_modules/gulp-cli/bin/gulp.js bundle:webpack
[22:55:43] Using gulpfile ~/work/arrow/js/gulpfile.js
[22:55:43] Starting 'bundle:webpack'...
[22:55:44] 'bundle:webpack' errored after 1.08 s
[22:55:44] Error in plugin "webpack-stream"
Message:
    Module not found: Error: Can't resolve 'apache-arrow' in '/home/xxx/work/arrow/js/test/bundle'

So I ran ./node_modules/.bin/gulp build:apache-arrow command additionally.
I could not reproduce the failure.

$ cd js
$ npm install
$ ./node_modules/.bin/gulp build:apache-arrow
$ node node_modules/gulp-cli/bin/gulp.js bundle:webpack
[23:04:30] Using gulpfile ~/work/arrow/js/gulpfile.js
[23:04:30] Starting 'bundle:webpack'...
[23:04:46] field-bundle.js: 13.93 kB (gzipped: 3.58 kB)
[23:04:46] makeTable-bundle.js: 69.61 kB (gzipped: 17 kB)
[23:04:46] makeVector-bundle.js: 55.97 kB (gzipped: 13.84 kB)
[23:04:46] schema-bundle.js: 13.93 kB (gzipped: 3.58 kB)
[23:04:46] table-bundle.js: 67.96 kB (gzipped: 16.67 kB)
[23:04:46] tableFromArrays-bundle.js: 86.06 kB (gzipped: 21.33 kB)
[23:04:46] tableFromIPC-bundle.js: 160.54 kB (gzipped: 35.81 kB)
[23:04:46] vector-bundle.js: 50.84 kB (gzipped: 13.07 kB)
[23:04:46] vectorFromArray-bundle.js: 77.64 kB (gzipped: 19.18 kB)
[23:04:46] Finished 'bundle:webpack' after 17 s

@kou Different node version?

# My Environment
$ node -v
v18.15.0

$ npm -v
9.7.1

@kou
Copy link
Member Author

kou commented Jun 12, 2023

I used system NodeJS v18.13.0 and npm 9.2.0 on Debian GNU/Linux sid:

Could you try this on Debian GNU/Linux bookwarm? It seems that it uses the same versions.

@abetomo
Copy link
Contributor

abetomo commented Jun 12, 2023

Thanks for the info.
I reproduced it in the following environment.

$ cat /etc/debian_version
12.0
$ node -v
v18.13.0
$ npm -v
9.2.0
$ node node_modules/gulp-cli/bin/gulp.js bundle:webpack
[11:51:54] Using gulpfile /work/arrow/js/gulpfile.js
[11:51:54] Starting 'bundle:webpack'...
[11:51:54] 'bundle:webpack' errored after 413 ms
[11:51:54] Error [ERR_STREAM_PREMATURE_CLOSE]: Premature close
    at new NodeError (node:internal/errors:400:5)
    at Stream.<anonymous> (node:internal/streams/pipeline:352:14)
    at Stream.emit (node:events:525:35)
    at Stream.emit (node:domain:552:15)
    at stream.destroy (/work/arrow/js/node_modules/through/index.js:84:12)
    at _end (/work/arrow/js/node_modules/through/index.js:67:14)
    at stream.end (/work/arrow/js/node_modules/through/index.js:74:5)
    at DestroyableTransform.onend (/work/arrow/js/node_modules/readable-stream/lib/_stream_readable.js:577:10)
    at Object.onceWrapper (node:events:627:28)
    at DestroyableTransform.emit (node:events:525:35)

I will try to fix it.

@abetomo
Copy link
Contributor

abetomo commented Jun 12, 2023

It seems to succeed in Node.js 18.14.0 or later.

@kou
Copy link
Member Author

kou commented Jun 13, 2023

Thanks.
Is it a bug of Node.js < 18.14? (Should we require 18.14 or later?) Or does our bundle:webpack have a problem for Node.js < 18.14?

@abetomo
Copy link
Contributor

abetomo commented Jun 13, 2023

It's a Node.js bug.
This failure is not a bundle:webpack problem.
It should build successfully on the latest Node.js 20 without any code changes.

@kou
Copy link
Member Author

kou commented Jun 13, 2023

I see.
Can you update our dev/release/verify-release-candidate.sh to require Node.js 16.* or 18.14+ by something like this?

diff --git a/dev/release/verify-release-candidate.sh b/dev/release/verify-release-candidate.sh
index 4183520a0..8aeb0f6b5 100755
--- a/dev/release/verify-release-candidate.sh
+++ b/dev/release/verify-release-candidate.sh
@@ -315,22 +315,22 @@ install_nodejs() {
     return 0
   fi
 
-  required_node_major_version=16
   node_major_version=$(node --version 2>&1 | grep -o '^v[0-9]*' | sed -e 's/^v//g' || :)
-
-  if [ -n "${node_major_version}" ] && [ "${node_major_version}" -ge ${required_node_major_version} ]; then
-    show_info "Found NodeJS installation with major version ${node_major_version}"
+  node_minor_version=$(node --version 2>&1 | grep -o '^v[0-9]*\.[0-9]*' | sed -e 's/^v[0-9]*\.//g' || :)
+  if [ -n "${node_major_version}" ] && [ -n "${node_minor_version}" ] && \
+       { [ "${node_major_version}" -eq 16 ] || \
+           { [ "${node_major_version}" -eq 18 ] && [ "${node_minor_version}" -ge 14 ] } || \
+           [ "${node_major_version}" -ge 20 ] }; then
+    show_info "Found Node.js installation with major version ${node_major_version}.${node_minor_version}"
   else
-    export NVM_DIR="`pwd`/.nvm"
+    export NVM_DIR="$(pwd)/.nvm"
     mkdir -p $NVM_DIR
     curl -sL https://raw.githubusercontent.com/nvm-sh/nvm/v0.39.0/install.sh | \
       PROFILE=/dev/null bash
     [ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh"
 
-    # ARROW-18335: "gulp bundle" failed with Node.js 18.
-    # nvm install --lts
-    nvm install 16
-    show_info "Installed NodeJS $(node --version)"
+    nvm install --lts
+    show_info "Installed Node.js $(node --version)"
   fi
 
   NODEJS_ALREADY_INSTALLED=1
@@ -837,7 +837,7 @@ test_js() {
   show_header "Build and test JavaScript libraries"
 
   maybe_setup_nodejs || exit 1
-  maybe_setup_conda nodejs=16 || exit 1
+  maybe_setup_conda nodejs=18 || exit 1
 
   if ! command -v yarn &> /dev/null; then
     npm install yarn

We can test dev/release/verify-release-candidate.sh by TEST_DEFAULT=0 TEST_JS=1 dev/release/verify-release-candidate.sh.

@domoritz
Copy link
Member

Thanks for making these updates to the CI!

@abetomo
Copy link
Contributor

abetomo commented Jun 14, 2023

I will update verify-release-candidate.sh and also work on #36055.

abetomo added a commit to abetomo/arrow that referenced this issue Jun 15, 2023
…candidate.sh`

Signed-off-by: abetomo <abe@enzou.tokyo>
kou pushed a commit that referenced this issue Jun 15, 2023
…ate.sh` (#36089)

### Rationale for this change

In Node.js 18.14 or later, `gulp bundle:webpack` succeeds.
So we support running `verify-release-candidate.sh` with Node.js 18.14 or later.

### What changes are included in this PR?

Node.js version check allows 16, 18.14 or later.
Install Node.js LTS with `nvm`.

### Are these changes tested?

I have tested that `TEST_DEFAULT=0 TEST_JS=1 dev/release/verify-release-candidate.sh` succeeds in several environments.

* Node.js is not installed
    * v18.16.0 installed with `nvm` and `verify-release-candidate.sh` succeeded.
* Node.js 16.x installed
    *  Node.js 16.x is used and `verify-release-candidate.sh` succeeded.
* Node.js 18.13.0 installed
    * v18.16.0 installed  with `nvm` and `verify-release-candidate.sh` succeeded.
* Node.js 18.14.0 installed
    *  Node.js 18.14.0 is used and `verify-release-candidate.sh` succeeded.
* Node.js 20.x installed
    *  Node.js 20.x is used and `verify-release-candidate.sh` succeeded.

### Are there any user-facing changes?

* Closes: #36025

Authored-by: abetomo <abe@enzou.tokyo>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@kou kou added this to the 13.0.0 milestone Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants