-
Notifications
You must be signed in to change notification settings - Fork 1
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
Bootstrapping the initiative: TODOs? #2
Comments
cc @nodejs/tsc @nodejs/loaders for visibility |
Planning to discuss it at the collaboration summit session openjs-foundation/summit#422 |
There has been some related discussion in the @nodejs/package-maintenance team so at mentioning team as an FYI |
Hello, I attended the early morning session remotely, and I’m currently working on maintaining the react-leaflet-markercluster repository, which was archived for the past two years. One of my initial tasks was to implement ESM support to ensure compatibility with vitest. While I’ve made these adjustments, I’m uncertain if the changes maintain CJS compatibility. I’m currently reviewing resources on ESM/CJS configuration for Node.js, including this article, to solidify my understanding. Given the small scope of this repository and the few files changed, I’d appreciate a review to confirm my approach. I’m also happy to offer this repo as an example for the broader initiative on shipping patterns. Here is the PR: yuzhva/react-leaflet-markercluster#214 Thank you! |
@charlieforward9 I am not an expert on react but since the PR is using require + import conditions, you might want to give nodejs/node#52174 a read - if the package would be fine to have two instance of itself in the same context and relies on no global state, then it's probably okay. Otherwise it may run into the dual package hazard:
now a duplicate pair of the CJS and the ESM version of your package are loaded and initialized in the same context, which may or may not be okay. |
Notes from the collaboration summit session: https://hackmd.io/V3xtjlcrTIGsemPv8t-tKg |
Opened #3 for point 1: starting a team |
Removing agenda labels. Team discussions moved to #3 |
A few TODOs come to mind:
node
/default
conditions instead ofrequire
/import
as a solution to the dual package hazard node#52174 which recommendsnode
/default
overimport
/require
. We should look into documenting what the community actually does, and if necessary, document discussions about their pros and cons.require(esm)
is still experimental.The text was updated successfully, but these errors were encountered: