Skip to content

Conversation

@ChristopherPHolder
Copy link

@ChristopherPHolder ChristopherPHolder commented Sep 18, 2023

This MR addresses the issue from MR #49

image

@BioPhoton
Copy link
Collaborator

@ChristopherPHolder thx for having a look at this!

Did you try to execute the build artefact?
npx dist/packages/cli collect

Copy link
Collaborator

@BioPhoton BioPhoton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would a combination of bundle false + the additional entry point work for us?

@ChristopherPHolder
Copy link
Author

Would a combination of bundle false + the additional entry point work for us?

It is worth testing it.

@matejchalk
Copy link
Collaborator

I had to change the bin.js path, but otherwise this seems to work just fine 🙂

image

@BioPhoton
Copy link
Collaborator

Here another 🧩 of the puzzle:
https://github.com/favware/esbuild-plugin-file-path-extensions

@ChristopherPHolder
Copy link
Author

So it seems it is really not straight forward. Just to getting running for the moment i have converted the bin to a .js file and passed as an asset.

I know this is not optimal and i am looking into the configuration for getting the bundling correct without the issue of import file extensions.

@ChristopherPHolder
Copy link
Author

Relevent: evanw/esbuild#622

@matejchalk
Copy link
Collaborator

I'm OK with the bin.js asset as a workaround 🤷‍♂️

Although as mentioned in #73, maybe we don't need two entry points for the cli anyway, index.ts could just execute instead of only exporting a function. I'm not sure we need to support importing from cli, users that want programmatic usage should be able to import everything they need from core (or models, utils, etc.).

@ChristopherPHolder
Copy link
Author

I'm OK with the bin.js asset as a workaround 🤷‍♂️

Although as mentioned in #73, maybe we don't need two entry points for the cli anyway, index.ts could just execute instead of only exporting a function. I'm not sure we need to support importing from cli, users that want programmatic usage should be able to import everything they need from core (or models, utils, etc.).

I am in full agreement. What are your thoughts on this? @BioPhoton

@ChristopherPHolder ChristopherPHolder marked this pull request as ready for review September 29, 2023 13:54
@BioPhoton
Copy link
Collaborator

I'm in for the core package!
lets for now have only the bin.ts file present and merge the discussed approach for the cli package.
I'll setup a core package later.

@BioPhoton
Copy link
Collaborator

closed as implemented in #66

@BioPhoton BioPhoton closed this Oct 10, 2023
@BioPhoton BioPhoton deleted the add-bin-entry-point branch November 20, 2023 23:56
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.

4 participants