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] Remove bin/bin directory from bundles #34567

Closed
domoritz opened this issue Mar 14, 2023 · 9 comments · Fixed by #36607
Closed

[JS] Remove bin/bin directory from bundles #34567

domoritz opened this issue Mar 14, 2023 · 9 comments · Fixed by #36607

Comments

@domoritz
Copy link
Member

Describe the bug, including details regarding any error messages, version, and platform.

The generated bundles contain a /bin/bin directory that should not be there.

Screenshot 2023-03-14 at 18 23 03

Component(s)

JavaScript

@kou kou changed the title Remove bin/bin directory from bundles [JS] Remove bin/bin directory from bundles Mar 15, 2023
@abetomo
Copy link
Contributor

abetomo commented Jun 30, 2023

I haven't looked into it in detail, but the following patch may improve it.

--- a/js/gulp/typescript-task.js
+++ b/js/gulp/typescript-task.js
@@ -34,7 +34,6 @@ export const typescriptTask = ((cache) => memoizeTask(cache, function typescript
     const out = targetDir(target, format);
     const tsconfigPath = path.join(`tsconfig`, `tsconfig.${tsconfigName(target, format)}.json`);
     return compileTypescript(out, tsconfigPath)
-        .pipe(mergeWith(compileBinFiles(target, format)))
         .pipe(takeLast(1))
         .pipe(share({ connector: () => new ReplaySubject(), resetOnError: false, resetOnComplete: false, resetOnRefCountZero: false }))
 }))({});

@domoritz
Copy link
Member Author

We will need to check carefully since we want the bin files themselves included.

@abetomo
Copy link
Contributor

abetomo commented Jun 30, 2023

Is it a problem you need solved in a hurry?
May I work on it?

@domoritz
Copy link
Member Author

No rush. Go for it! It should be easy to see whether you succeeded by checking the bundles generated by npm pack. Thanks!

@abetomo
Copy link
Contributor

abetomo commented Jun 30, 2023

Thanks for the reply!
I got it.

@abetomo
Copy link
Contributor

abetomo commented Jun 30, 2023

take

@abetomo
Copy link
Contributor

abetomo commented Jul 4, 2023

Is it correct that the following files generated under targets should not exist?

targets/apache-arrow/bin/bin/arrow2csv.js
targets/apache-arrow/bin/bin/arrow2csv.js.map
targets/apache-arrow/bin/bin/arrow2csv.mjs
targets/apache-arrow/bin/src/bin/arrow2csv.ts
targets/es2015/cjs/bin/bin/arrow2csv.js
targets/es2015/cjs/bin/bin/arrow2csv.js.map
targets/es2015/cjs/bin/src/bin/arrow2csv.ts
targets/es2015/esm/bin/bin/arrow2csv.js
targets/es2015/esm/bin/bin/arrow2csv.js.map
targets/es2015/esm/bin/src/bin/arrow2csv.ts
targets/es2015/umd/bin/bin/arrow2csv.js
targets/es2015/umd/bin/bin/arrow2csv.js.map
targets/es2015/umd/bin/src/bin/arrow2csv.ts
targets/es5/cjs/bin/bin/arrow2csv.js
targets/es5/cjs/bin/bin/arrow2csv.js.map
targets/es5/cjs/bin/src/bin/arrow2csv.ts
targets/es5/esm/bin/bin/arrow2csv.js
targets/es5/esm/bin/bin/arrow2csv.js.map
targets/es5/esm/bin/src/bin/arrow2csv.ts
targets/es5/umd/bin/bin/arrow2csv.js
targets/es5/umd/bin/bin/arrow2csv.js.map
targets/es5/umd/bin/src/bin/arrow2csv.ts
targets/esnext/cjs/bin/bin/arrow2csv.js
targets/esnext/cjs/bin/bin/arrow2csv.js.map
targets/esnext/cjs/bin/src/bin/arrow2csv.ts
targets/esnext/esm/bin/bin/arrow2csv.js
targets/esnext/esm/bin/bin/arrow2csv.js.map
targets/esnext/esm/bin/src/bin/arrow2csv.ts
targets/esnext/umd/bin/bin/arrow2csv.js
targets/esnext/umd/bin/bin/arrow2csv.js.map
targets/esnext/umd/bin/src/bin/arrow2csv.ts

@domoritz
Copy link
Member Author

domoritz commented Jul 4, 2023

I think so, yes. We should make sure the scripts still work and that the sources are still in a location where the editor can give you correct sources.

@abetomo
Copy link
Contributor

abetomo commented Jul 11, 2023

5598d2f#diff-cef18da59d581d0c92a1e262b6e7de2afac26d08cfa38fd8917f12fa9c48960fL2-R2

js/tsconfig/tsconfig.base.json

-  "exclude": ["../node_modules", "../src/bin/*.ts"],
+  "exclude": ["../node_modules"],

This change seems to generate bin without compileBinFiles(#34567 (comment)).

abetomo added a commit to abetomo/arrow that referenced this issue Jul 11, 2023
…ctory

Signed-off-by: abetomo <abe@enzou.tokyo>
abetomo added a commit to abetomo/arrow that referenced this issue Jul 16, 2023
…ctory

Signed-off-by: abetomo <abe@enzou.tokyo>
trxcllnt pushed a commit to abetomo/arrow that referenced this issue Sep 14, 2023
…ctory

Signed-off-by: abetomo <abe@enzou.tokyo>
domoritz pushed a commit that referenced this issue Sep 15, 2023
…36607)

### Rationale for this change

`bin/bin` directory is unnecessary and should not be generated.

### What changes are included in this PR?

* Add setting to exclude in tsconfig
* Correctly set up `bin` out directory

### Are these changes tested?

The following files are not generated.

```
targets/apache-arrow/bin/bin/arrow2csv.js
targets/apache-arrow/bin/bin/arrow2csv.js.map
targets/apache-arrow/bin/bin/arrow2csv.mjs
targets/apache-arrow/bin/src/bin/arrow2csv.ts
targets/es2015/cjs/bin/bin/arrow2csv.js
targets/es2015/cjs/bin/bin/arrow2csv.js.map
targets/es2015/cjs/bin/src/bin/arrow2csv.ts
targets/es2015/esm/bin/bin/arrow2csv.js
targets/es2015/esm/bin/bin/arrow2csv.js.map
targets/es2015/esm/bin/src/bin/arrow2csv.ts
targets/es2015/umd/bin/bin/arrow2csv.js
targets/es2015/umd/bin/bin/arrow2csv.js.map
targets/es2015/umd/bin/src/bin/arrow2csv.ts
targets/es5/cjs/bin/bin/arrow2csv.js
targets/es5/cjs/bin/bin/arrow2csv.js.map
targets/es5/cjs/bin/src/bin/arrow2csv.ts
targets/es5/esm/bin/bin/arrow2csv.js
targets/es5/esm/bin/bin/arrow2csv.js.map
targets/es5/esm/bin/src/bin/arrow2csv.ts
targets/es5/umd/bin/bin/arrow2csv.js
targets/es5/umd/bin/bin/arrow2csv.js.map
targets/es5/umd/bin/src/bin/arrow2csv.ts
targets/esnext/cjs/bin/bin/arrow2csv.js
targets/esnext/cjs/bin/bin/arrow2csv.js.map
targets/esnext/cjs/bin/src/bin/arrow2csv.ts
targets/esnext/esm/bin/bin/arrow2csv.js
targets/esnext/esm/bin/bin/arrow2csv.js.map
targets/esnext/esm/bin/src/bin/arrow2csv.ts
targets/esnext/umd/bin/bin/arrow2csv.js
targets/esnext/umd/bin/bin/arrow2csv.js.map
targets/esnext/umd/bin/src/bin/arrow2csv.ts
```

### Are there any user-facing changes?

* Closes: #34567

Lead-authored-by: abetomo <abe@enzou.tokyo>
Co-authored-by: ptaylor <paul.e.taylor@me.com>
Signed-off-by: Dominik Moritz <domoritz@gmail.com>
@domoritz domoritz added this to the 14.0.0 milestone Sep 15, 2023
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue Nov 13, 2023
…ctory (apache#36607)

### Rationale for this change

`bin/bin` directory is unnecessary and should not be generated.

### What changes are included in this PR?

* Add setting to exclude in tsconfig
* Correctly set up `bin` out directory

### Are these changes tested?

The following files are not generated.

```
targets/apache-arrow/bin/bin/arrow2csv.js
targets/apache-arrow/bin/bin/arrow2csv.js.map
targets/apache-arrow/bin/bin/arrow2csv.mjs
targets/apache-arrow/bin/src/bin/arrow2csv.ts
targets/es2015/cjs/bin/bin/arrow2csv.js
targets/es2015/cjs/bin/bin/arrow2csv.js.map
targets/es2015/cjs/bin/src/bin/arrow2csv.ts
targets/es2015/esm/bin/bin/arrow2csv.js
targets/es2015/esm/bin/bin/arrow2csv.js.map
targets/es2015/esm/bin/src/bin/arrow2csv.ts
targets/es2015/umd/bin/bin/arrow2csv.js
targets/es2015/umd/bin/bin/arrow2csv.js.map
targets/es2015/umd/bin/src/bin/arrow2csv.ts
targets/es5/cjs/bin/bin/arrow2csv.js
targets/es5/cjs/bin/bin/arrow2csv.js.map
targets/es5/cjs/bin/src/bin/arrow2csv.ts
targets/es5/esm/bin/bin/arrow2csv.js
targets/es5/esm/bin/bin/arrow2csv.js.map
targets/es5/esm/bin/src/bin/arrow2csv.ts
targets/es5/umd/bin/bin/arrow2csv.js
targets/es5/umd/bin/bin/arrow2csv.js.map
targets/es5/umd/bin/src/bin/arrow2csv.ts
targets/esnext/cjs/bin/bin/arrow2csv.js
targets/esnext/cjs/bin/bin/arrow2csv.js.map
targets/esnext/cjs/bin/src/bin/arrow2csv.ts
targets/esnext/esm/bin/bin/arrow2csv.js
targets/esnext/esm/bin/bin/arrow2csv.js.map
targets/esnext/esm/bin/src/bin/arrow2csv.ts
targets/esnext/umd/bin/bin/arrow2csv.js
targets/esnext/umd/bin/bin/arrow2csv.js.map
targets/esnext/umd/bin/src/bin/arrow2csv.ts
```

### Are there any user-facing changes?

* Closes: apache#34567

Lead-authored-by: abetomo <abe@enzou.tokyo>
Co-authored-by: ptaylor <paul.e.taylor@me.com>
Signed-off-by: Dominik Moritz <domoritz@gmail.com>
dgreiss pushed a commit to dgreiss/arrow that referenced this issue Feb 19, 2024
…ctory (apache#36607)

### Rationale for this change

`bin/bin` directory is unnecessary and should not be generated.

### What changes are included in this PR?

* Add setting to exclude in tsconfig
* Correctly set up `bin` out directory

### Are these changes tested?

The following files are not generated.

```
targets/apache-arrow/bin/bin/arrow2csv.js
targets/apache-arrow/bin/bin/arrow2csv.js.map
targets/apache-arrow/bin/bin/arrow2csv.mjs
targets/apache-arrow/bin/src/bin/arrow2csv.ts
targets/es2015/cjs/bin/bin/arrow2csv.js
targets/es2015/cjs/bin/bin/arrow2csv.js.map
targets/es2015/cjs/bin/src/bin/arrow2csv.ts
targets/es2015/esm/bin/bin/arrow2csv.js
targets/es2015/esm/bin/bin/arrow2csv.js.map
targets/es2015/esm/bin/src/bin/arrow2csv.ts
targets/es2015/umd/bin/bin/arrow2csv.js
targets/es2015/umd/bin/bin/arrow2csv.js.map
targets/es2015/umd/bin/src/bin/arrow2csv.ts
targets/es5/cjs/bin/bin/arrow2csv.js
targets/es5/cjs/bin/bin/arrow2csv.js.map
targets/es5/cjs/bin/src/bin/arrow2csv.ts
targets/es5/esm/bin/bin/arrow2csv.js
targets/es5/esm/bin/bin/arrow2csv.js.map
targets/es5/esm/bin/src/bin/arrow2csv.ts
targets/es5/umd/bin/bin/arrow2csv.js
targets/es5/umd/bin/bin/arrow2csv.js.map
targets/es5/umd/bin/src/bin/arrow2csv.ts
targets/esnext/cjs/bin/bin/arrow2csv.js
targets/esnext/cjs/bin/bin/arrow2csv.js.map
targets/esnext/cjs/bin/src/bin/arrow2csv.ts
targets/esnext/esm/bin/bin/arrow2csv.js
targets/esnext/esm/bin/bin/arrow2csv.js.map
targets/esnext/esm/bin/src/bin/arrow2csv.ts
targets/esnext/umd/bin/bin/arrow2csv.js
targets/esnext/umd/bin/bin/arrow2csv.js.map
targets/esnext/umd/bin/src/bin/arrow2csv.ts
```

### Are there any user-facing changes?

* Closes: apache#34567

Lead-authored-by: abetomo <abe@enzou.tokyo>
Co-authored-by: ptaylor <paul.e.taylor@me.com>
Signed-off-by: Dominik Moritz <domoritz@gmail.com>
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.

2 participants