-
Notifications
You must be signed in to change notification settings - Fork 163
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
Add ESM and rollup build #140
base: master
Are you sure you want to change the base?
Conversation
IMHO you have added a bit too much new pretty-print formatting, making the diff larger than it has to be. Can you reduce that a bit to zone in on just the top-level? |
Yeah, sure. I admit some of it was my editor auto-formatting by itself. What makes the diff difficult is that the source file is now I've pushed a new version that minimizes formatting changes. The diff is still weird, but that is due to the new file not being compared with the old source. Comparing |
rollup.config.mjs
Outdated
export default defineConfig([ | ||
{ | ||
input: 'src/fuzzysort.js', | ||
output: { banner, exports, file: pkg.browser, format: 'umd', name: 'fuzzysort' }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while you're at it I suggest also enabling source map output to help debugging.
https://rollupjs.org/configuration-options/#output-sourcemap
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For just the minified file? Or all? The sourcemap is quite big to be checked in IMO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mean check in, but generate in build. At least there's no source map comment at the end of the fuzzsort.min.js file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But the build output is checked in now. I didn't want to change that compared to how it is now. If I enable sourcemaps a .map file would have to be checked in for each file, since the output is also checked in
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mm, maybe a compromise is to add flag to generate .map files but don't check them in? With reasonable assumption users will run a real build themselves.
And change back to the non-minified version in the test.html for the quick tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. I've enabled sourcemaps and added the maps to the gitignore. Adding the non-minified version for the tests is a bit tricky since the non-minified versions are ESM and CJS, not IIFE format. Debugging does work great with sourcemaps and the minified files now
When comparing to the /src version it's much better now yes. |
Those are added to the build output. That way the version number is always up-to-date |
ah, ok |
Fixes farzher#94 Adds a rollup config file to build ESM, CJS, and UMD bundles. Based on the source in src/fuzzysort.js. Checks in the new esm file and the updated cjs and umd files. Also updates the package.json to include `module` and `browser` fields for the ESM and UMD bundles.
@farzher could you take a look? 🙏 |
First of all, thanks for this awesome library!
Fixes #94
Adds a rollup config file to build ESM, CJS, and UMD bundles. Based on the source in src/fuzzysort.js.
Checks in the new esm file and the updated cjs and umd files.
Also updates the package.json to include
module
andbrowser
fields for the ESM and UMD bundles. This lets bundlers, node or CDN's know which version to use.I've tried to keep things fairly streamlined. Running 'test' automatically builds the files. The
src/fuzzysort.js
andfuzzysort.mjs
files are extremely similar, but that seemed better than the confusion of knowing which file in the root to edit and having rollup potentially overwrite your changes. I've also kept the build files checked in, to keep things simple. A nice future improvement I'm willing to contribute would be github actions CI and creating a PR when the files drift?