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

Fix: sources should properly require dependencies for Node compatibility #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joshgoebel
Copy link
Contributor

@joshgoebel joshgoebel commented Apr 25, 2022

Creating this largely as a discussion topic (since issues aren't open). Can dependencies please be handled properly such that using this from nodejs "just works" - like with every NPM libraries - ie, libraries (in pre-bundled form) should require their own dependencies... Users should not be forced to rely on globals just to use the library from Node proper.

I do think the final distributable would be improved if it also bundled colorparsley (a tiny dependency) rather than forcing client-side users to deal with dependency hell, but that's a secondary issue.


Also: Can the fact that this is a template repo be fixed and could issues be opposed to allow people to file issues?

Creating this largely as a discussion topic (since issues aren't open).    Can dependencies please be handled properly such that using this from `nodejs` "just works" - like with every NPM libraries - ie, libraries (in raw form) should require their own dependencies...  Users should not be forced to rely on globals just to use the library from Node proper.

I do think the final distributable would be improved if it also bundled `colorparsley` (a tiny dependency) rather than forcing client-side users to deal with dependency hell, but that's a secondary issue.

---

Also: Can the fact that this is a template repo be fixed and could issues be opposed to allow people to file issues?
@joshgoebel joshgoebel changed the title Main sources should properly require dependencies Main sources should properly require dependencies for Node compatibility Apr 25, 2022
@joshgoebel joshgoebel changed the title Main sources should properly require dependencies for Node compatibility Fix: sources should properly require dependencies for Node compatibility Apr 25, 2022
@Myndex
Copy link
Owner

Myndex commented Apr 25, 2022

Hi Josh @joshgoebel

Thank you! Just FYI, discussions are open at the MAIN repository, which is https://github.com/Myndex/SAPC-APCA/discussions

And the more complete documentation is in https://github.com/Myndex/SAPC-APCA/tree/master/documentation

I suppose I should make that more clear!

McCoy Dammit Jim I'm a researcher not a javascript guru!

LOL.

This project, and the related colorparsley, are the first projects I've published on npm.

As it started as a client side script, it may not have everything in pace for node.js, though I am working to make that a smoother experience.

I was under the impression if I listed the dependency in the package.json that it would be included, though I see my import statement was commented out at compile time, and I see that you've suggested using the require sttement instead.

Do I need to change the extension from .js to .cjs?

I think this is partly due to how I have my dev environment setup for GitHub desktop and npm. I am happy to entertain suggestions to help improve compatibility—mainly to shift my dev environment from client-side oriented to node. Any resources you might suggest?

Thank you for your help!

Andy

@joshgoebel
Copy link
Contributor Author

joshgoebel commented Apr 25, 2022

This project, and the related colorparsley, are the first projects I've published on npm.

Welcome. :-)

I was under the impression if I listed the dependency in the package.json that it would be included

No, that just gets it installed... you still need to require/import it when it's needed. (for Node.js)

Do I need to change the extension from .js to .cjs? ... mainly to shift my dev environment from client-side oriented to node

Well, it's kind of a mess right now unless you embrace ESM everywhere... (making it harder for some to use your library IMHO). You might read: sindresorhus/meta#15 or google ESM vs CJS

Right now Highlight.js publishes:

  • browser/cdn builds (global monolith and ESM)
  • node build (CJS and ESM)

So we turn one code base into 3 different use cases/builds for people:

  • those who want to require (CJS, the tried and true)
  • those who want to import on client or server (ESM, the future)
  • those who just want a few <script> tags to magically work (monolith build, window.hljs global, etc)

Our core library is all clean ESM code that's compiled to whatever module type is needed. However our build tools/scripts are current still all CJS... so if you went ESM only that would mean we couldn't use your library. For Summer 2022 we're considering going ESM only and dropping our CJS Node build (but not the browser monoliths).

Sorry I'm not more help.

@joshgoebel
Copy link
Contributor Author

joshgoebel commented Apr 25, 2022

If your library is mostly self-contained (as it seems to be) I'd recommend publishing a browser-ready JS file that is "all batteries included" (meaning it compiles in the colorparsley dependency)... if your goal is to make things easy for people.
Then you're just left debating how you want to deal with CJS or ESM on the server-side... Personally for build tools I love rollup. https://rollupjs.org/guide/en/

@joshgoebel
Copy link
Contributor Author

joshgoebel commented Apr 25, 2022

Just FYI, discussions are open at the MAIN repository,

Personally I still think issues should be open here (for issues related to the npm library/source specifically not the larger project), but that's just my opinion - and what I think is most common... some people/large projects have different ideas on these things. :)

@Myndex
Copy link
Owner

Myndex commented Apr 25, 2022

Hey @joshgoebel thank you for the additional thoughts and resources.

Because of the overall goal and the nature of the project as a emerging international standard, I want to make apca-w3 and Bridge-PCA as easy to implement and compatible as possible, so thank you for the tip on using rollup...

And yes, apca-w3 is now self-contained, with the only dependency being colorparsley which is used by the entire rest of the family.

I opened the discussions tab instead of issues as it seems to be the only way to pin a thread that indicates the main repo is elsewhere, and what should be there vs here.

Thank you!
A

@Myndex
Copy link
Owner

Myndex commented Jun 20, 2022

Hi @joshgoebel I just started a branch called BuildTools and am integrating the work Alex did, updating to the new functions of course, hope to have these published this week.

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