itemsapi / itemsjs

Extremely fast faceted search engine in JavaScript - lightweight, flexible, and simple to use
Apache License 2.0
346 stars 41 forks source link

Converted to ES modules #135

Closed stereobooster closed 10 months ago

stereobooster commented 10 months ago

Not sure if you are interested (if not feel free to close).

As the result build size reduced from 100kb to 10kb

Some speed improvements:

PS recently wrote an article about front-end faceted search and apparently there are only 3 options

cigolpl commented 10 months ago

I've taken some time to read through the article you contributed, and I wanted to thank you for the thorough analysis.
While I haven't used TypeScript extensively before, I understand the benefits and I'm very pleased with your PR. I especially appreciate the significant reduction in the dist file size this will definitely benefit our end users. The updates to the libraries and the automated test system are working really well in my tests.

As we're moving forward, I've encountered a hiccup with our JSFiddle example. It's currently not operating as expected with the updated library. I've updated the script source to use index.umd.js (https://6548b30bf4597f01be4be6ae--heartfelt-pithivier-087d23.netlify.app/index.umd.js) instead of itemsjs.min.js, but it's throwing a TypeError in the console:

fulltext.ts:23 Uncaught TypeError: o.default is not a function
    at new t (fulltext.ts:23:17)
    at index.ts:27:36
    at (index):188:11

The example is accessible here: https://jsfiddle.net/cigol/0ef9qeos/196/. Could you have a look and advise on any modifications that might resolve this?

In addition, I'm proposing a small tweak for the clone function to maintain compatibility with Node.js 16, presented here:

export function clone<T>(val: T): T {
  try {
    // Try to use structuredClone if available.
    return structuredClone(val);
  } catch (e) {
    // If structuredClone is not available or fails, use JSON-based cloning as a fallback.
    return JSON.parse(JSON.stringify(val));
  }
}

Including this fallback in the PR would help us ensure a wider compatibility range.

Your input and contribution are very much valued, and I'm looking forward to any suggestions you might have.

stereobooster commented 10 months ago

While I haven't used TypeScript extensively before, I understand the benefits and I'm very pleased with your PR

To be sure we're on the same side. This PR doesn't use TypeScript. Maybe you checked out main branch of my fork. You want to check js branch instead.

I didn't pushed TS-version as PR, because thought it would be way too much changes at once

o.default is not a function

Microbundle can't resolve properly lunr package, which itself is published as UMD. That is tricky 🤔. I will need to check if there is a way around

In addition, I'm proposing a small tweak for the clone function to maintain compatibility with Node.js 16, presented here:

Sure

stereobooster commented 10 months ago

One way would be to convert lunr.js to ES modules as well, but it seems library haven't been updated in 3 years (https://github.com/olivernn/lunr.js/issues/504). Even if I make a PR there, it probably won't be accepted

stereobooster commented 10 months ago

There are 3 options (which I can think of):

cigolpl commented 10 months ago

Yes, I was testing the TypeScript branch before (with vitest as testing environment)

Thank you for the suggestions on how to address the issue we're facing.

Fixing the issue in microbundle: This would be the ideal solution, as it directly addresses the root of the problem. However, this is also the option with the least predictability since it relies on external maintainers.

Refusing from UMD distribution to use ES modules in the browser: It's worth noting that while modern browsers support ES modules, a complete shift might not accommodate a significant segment of developers who continue to use UMD. Many libraries still support UMD, which suggests that it remains a considerable part of the JavaScript ecosystem. Without exact market data, it's challenging to quantify this, but it's clear that UMD usage is still extensive.

Switching to an alternative library: I propose Minisearch. It's is a lightweight library that supports ES modules and has come up multiple times in our issue tracker. It seems to offer the functionalities we need for our search capabilities.

stereobooster commented 10 months ago

Kind people from microbundle helped me to understand issue. microbundle doesn't "pack" dependencies by default, which makes sense - if you install package via npm all those dependencies would be installed as well. So if I move dependencies (lunr etc.) microbundle will pack them in final js. But now size of final package is bigger

56K index.cjs
55K index.modern.js
56K index.module.js
56K index.umd.js

still smaller than it was, though

cigolpl commented 10 months ago

Excellent work! The size of the compressed file is now two times smaller than before, which is a fantastic improvement.