-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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] The package.json file incorrectly specifies sideEffects: false #38936
Comments
Good catch. Do you have an idea how we can test this to make sure we are doing the right fix and don't break it again? |
I am not sure what a good test would be here. I think a minimally-reproducible case might be to build a simple script like
using WebPack, and then run the result and assert that exit code is 0. But this is admittedly painful to put into a CI test suite. If you're interested in a test like this, I could put together the example for you. |
We already have bundler tests in https://github.com/apache/arrow/blob/b522f8c5cc152ea97d33340e3bef852e063dad46/js/test/bundle, actually. They are run in https://github.com/apache/arrow/blob/b522f8c5cc152ea97d33340e3bef852e063dad46/js/gulp/bundle-task.js and we already run the bundle in Line 129 in b522f8c
|
Apologies for the delay here. I've added a failing test in this PR: #39953 Let me know if there's anything else I can do to support this fix. |
I'm pretty swamped with things right now and we won't ship a fix until the next release (arrow 16) anyway so I'll put it in my queue. I think the approach of setting side effects in package.json is not good since it blows up bundle sizes so we should look into initializing in a class. |
Hey @domoritz, is there an ETA on when a patch for this may be available? It sounds like it's not considered worthy a new release, is there any timeline on v16 being published? Just wondering as this is kind of a blocker for some of our work due to no workaround being available when using esbuild. Appreciate all the help! |
All arrow packages I'm this repo are released together every three months. This is an important issue but I haven't found a solution yet. |
Describe the bug, including details regarding any error messages, version, and platform.
Problem
When building a Node application with Webpack, calls to
RecordBatchStreamWriter.toNodeStream
fail with the error:Cause
The
apache-arrow
package.json file specifies"sideEffects": false
, but the Node overrides required for streaming Arrow files are applied as a side effect. I believe this is true for the DOM overrides as well.Suggested Fix
I haven't tested this, but I think that wrapping the overrides in these two files in an IIFE as is done here will fix this issue in the Webpack build.
Component(s)
JavaScript
The text was updated successfully, but these errors were encountered: