lunr.js icon indicating copy to clipboard operation
lunr.js copied to clipboard

Incompatible with ES6 Modules

Open indolering opened this issue 5 years ago • 18 comments

I'm unable to load Lunr.js in an ES6 module. I know Lunr isn't packaged up in an ES6 module, but imported scripts will still cause side effects and the window.lunr global can be used within an ES6 module [stack overflow] [codepen example]:

import "https://unpkg.com/lunr/lunr.js"

var documents = [{ 
  "name": "Lunr",
  "text": "Like Solr, but much smaller, and not as bright."
}, {
  "name": "React",
  "text": "A JavaScript library for building user interfaces."
}, {
  "name": "Lodash",
  "text": "A modern JavaScript utility library delivering modularity, performance & extras."
}]

const lunr = window.lunr

var idx = lunr(function () {
  this.ref('name')
  this.field('text')

  documents.forEach(function (doc) {
    this.add(doc)
  }, this)
})

console.log(idx.search("bright"))

This results in an error: Uncaught TypeError: Cannot set property 'lunr' of undefined. on line 3461 root.lunr = factory()

indolering avatar May 22 '19 06:05 indolering

I struggle to keep up with the changes in the JavaScript ecosystem these days, I'm more than happy to get lunr to a state where it can just work with ES6 modules but I'm not sure where to start.

Perhaps you can educate me, what is the right way of allowing lunr to be used as an ES6 module, is it possible to do so whilst maintaining backwards compatibility with plain script tags as well as npm?

olivernn avatar May 22 '19 18:05 olivernn

@olivernn For sure, much more important that it work as a plain script and CDN.

cekvenich avatar May 23 '19 23:05 cekvenich

I would rework it as an ES6 module and let Babel handle transpiling it for legacy systems.

much more important that it work as a plain script and CDN.

Just publish a lunr.es6.js build alongside everything else.

indolering avatar May 24 '19 07:05 indolering

For 50 extra internet points, I will do the tedious work of converting the codebase to a modern, class based conventions.

indolering avatar May 24 '19 07:05 indolering

@indolering Seems like a step back, less useful. How many points for you to not to do something that requires extra steps and can't be used by all browsers.

cekvenich avatar May 24 '19 10:05 cekvenich

I did take a look at completely overhauling the build process a while ago, as part of this I wanted to migrate to rollup.js for bundling the various components together. I remember reading through how D3 migrated to ES6 modules. If I remember correctly I got stuck with at something (I'll try and dig up the issue/question) and couldn't get any help so progress stalled.

@cekvenich not breaking consumers is absolutely paramount in this regard, I have a feeling this is why progress stalled. My point of view is that all of this should be completely invisible to consumers.

olivernn avatar May 27 '19 18:05 olivernn

Seems like a step back, less useful. How many points for you to not to do something that requires extra steps and can't be used by all browsers.

I'm not up for breaking other people's code, Babel will happily transpile everything for legacy clients. 😸 .

indolering avatar May 31 '19 07:05 indolering

I experimented with Lebab (Babel spelled backwards) and it eliminates most of the grunt work wrt to switching to classes. However, since the modules are simply concatenated together, I ran into some trouble tracking dependencies and getting unit tests to work.

indolering avatar Jun 12 '19 02:06 indolering

My dev machine melted down, and I've only gotten around to this in the past week.

Because everything is partitioned into semantically disconnected files and later concatenated, it's difficult to incrementally make major changes. It would be easier to start with the final source file and work backwards, but that would make for a very messy git log : P

@olivernn I removed your custom module code, inserted export lunr and used a simple rollup config to generate a UMD and ES6 module. The UMD file passed all of the browser and Node.js unit tests. Rollup needs the concatenated file, would you prefer a temp file of just pipe the results around?

indolering avatar Jul 20 '19 08:07 indolering

@indolering I'm not sure what you mean? Do you have an example?

olivernn avatar Jul 24 '19 19:07 olivernn

I just used a temp file, you can see the results here: https://github.com/indo-dev-0/lunr.js

indo-dev-0 avatar Jul 28 '19 06:07 indo-dev-0

Hi, I converted multiple packages to ES Modules while maintaining backwards-compatibility. Do you still need help with this task ?

Also I would avoid other syntax changes (such as using class) to make the transition as smooth as possible. Further changes could be done in later commits.

GrosSacASac avatar Oct 03 '19 10:10 GrosSacASac

Do you still need help with this task ?

@GrosSacASac I submitted a PR but it bundles a few changes together. Separating them out as individual PR's might help, but it seems like @olivernn is just busy :(

Also I would avoid other syntax changes (such as using class) to make the transition as smooth as possible. Further changes could be done in later commits.

Yeah, I abandoned that project as doing so incrementally was not feasible.

indolering avatar Oct 14 '19 04:10 indolering

What are the next steps to resolve this issue ?

GrosSacASac avatar Oct 18 '19 13:10 GrosSacASac

ping @olivernn consider adding contributors to the repo so we don't have to wait

GrosSacASac avatar Dec 06 '19 13:12 GrosSacASac

@olivernn , thanks to this lib I could create the plugin that I wanted.. 😁 Hi. Is this not maintained anymore? Not complaining, just asking..

I copied the main file and just changed the export so I can use it like ES6 Modules.. see here. In case someone else needs it.

emersonbottero avatar Sep 10 '22 01:09 emersonbottero

A very simple workaround here is to modify the end of the file:

/**
 * export the module via AMD, CommonJS or as a browser global
 * Export code from https://github.com/umdjs/umd/blob/master/returnExports.js
 */
;(function (root, factory) {
  // ...
}(this, function () {
  //...
}))

In this snippet, we're calling this which is always undefined in scripts loaded as ES modules. Loaded as a classic script, this evaluates to window, but we don't want to sub that value in here because then node won't work. Instead, replace this with globalThis for a consistent cross-platform solution.

andy-blum avatar Feb 29 '24 16:02 andy-blum