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

refactor: simplify cjs/esm build, add initial testing #168

Merged
merged 12 commits into from
Sep 16, 2024

Conversation

Cherry
Copy link
Contributor

@Cherry Cherry commented Sep 14, 2024

Fixes #166
Closes #163
Closes #35

This removes the complex build setup here and switches to using tsup for quick bundling for dual cjs/esm. Rather than trying to maintain a bespoke solution here with individual CJS/ESM configs and custom build scripts, I figure it'd be easier to use a tried and tested solution in tens of thousands of other projects,

It also adds some very basic tests to validate that this can be imported in both CJS and ESM, which should hopefully prevent any regressions in future. I also added the tests @rtbenfield did for DO Storage Instrumentation in #35 to kick off some testing.

These can be extended in future with even more tests of course.

I tested this in a couple of my own projects and it seems to work great, with a much simpler and easier to maintain output, but would appreciate another critical eye.

@@ -20,9 +20,7 @@ jobs:
- uses: actions/checkout@v4
with:
fetch-depth: 0
- uses: pnpm/action-setup@v3
with:
version: 8
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the version here and in other workflows because this action will just read from the packageManager field in package.json, so no need to duplicate this.

@Cherry Cherry changed the title refactor: simplify cjs/esm build to modern tooling refactor: simplify cjs/esm build to modern tooling, add initial testing Sep 14, 2024
@Cherry Cherry changed the title refactor: simplify cjs/esm build to modern tooling, add initial testing refactor: simplify cjs/esm build, add initial testing Sep 14, 2024
@evanderkoogh
Copy link
Owner

OMG man.. I was dreading sitting down today with my laptop because I knew I had to wade through this entire mess of ESM/CJS crap trying to get everything to work again..

I owe you more than a few rounds of beer for fixing this next time I am in the UK :)

@evanderkoogh evanderkoogh merged commit ea41c94 into evanderkoogh:main Sep 16, 2024
1 check passed
@rdooley rdooley mentioned this pull request Sep 16, 2024
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.

Missing ./dist/esm/index.js etc. built files
2 participants