fix: LRU constructor

[wellwelwel]

@sidorares, this time, the fix is in JavaScript 🤹🏻‍♀️

---

This fixes the LRU Constructor:

• TypeError: LRU is not a constructor #1885

The problem is a compatibility issue between CommonJS and ECMAScript Modules when importing the lru-cache.

---

To fix it, I changed the import from LRU in ./lib/parser_cache.js and ./lib/connection.js:

• from

  const LRU = require('lru-cache');

• to

  const LRU = require('lru-cache').default;

Now, it's compatible with both CJS and ESM.

---

NextJS

I tested this fix in a latest NextJS project and it works perfectly without use the experimental: serverComponentsExternalPackages: ["mysql2"].

• It may be necessary to delete the ./.next cache folder before the first npm run build | dev.

---

Notes

• As I didn't tested in older versions of Node.js, the CI tests from this PR will be the "final test"
• In case some CI fail, I will close this PR (not today) 🥷🏻✨

[sidorares]

great work @wellwelwel

I wonder how difficult would it be to simulate the original problem. Just loading mysql2 from mjs script works fine without this change, and also I believe bundling with webpack also works. Do you think its possible to add some sort of a test case?

Not directly related, but still mentioned few times in relation to LRU:

• lru-cache version 9 has a breaking change, now instead of default export ( or CJS const LRU = require('lru-cache') ) its now a named export ( import { LRUCache } from 'lru-cache' / const { LRUCache } = require('lru-cache') ). Changing to a named export would probably fix this issue as well
• starting from version 8 it uses syntax incompatible with node 14 ( also changed in the package.json engine ) . I should have made upgrade a major version bump and also indicate in mysql2 engine > 14. There are some users who want to be able to continue using mysql2 in node 14, currently except for LRU nothing else requires dropping v14 support. Few options here: 1) admit v14 os past end of life, those who need it would have to stay on a later versions. Possibly upgrade to [email protected] and add engine > 14, also release mysql2 as 4.x 2) downgrade to version 7, with the fix from your PR to help with various ESM / CJS edge cases, probably add engine >= 14 and release as minor version

[sidorares]

actually its node 12 that is incompatible, due to this syntax

from https://node.green

also AbortController used to be polyfilled and now its not

https://nodejs.org/dist/latest-v18.x/docs/api/globals.html#class-abortcontroller

[wellwelwel]

@sidorares, thanks 🙋🏻‍♂️

---

Before I decide for .default, I tried the recommended from lru-cache documentation:

const { LRUCache: LRU } = require('lru-cache');

So it worked perfectly in NextJS and ESM, but not in CJS.

I understand that the require can resolves this missing .default, but the import doesn't.

So, I do this:

const { default: LRU } = require('lru-cache');

And now, it works for everyone 🤹🏻‍♀️

I decided to use .default for compatibility reasons.

---

To test it based in an error from NextJS, the only way I know, is simulating a project, but I don't think that it's a good way.
By example, an user reported the same error in NextJS, other in nuxt and another indicating that in NextJS 13.0 it works.
For these reasons, I believe that it's difficult to maintain automated tests on third-party frameworks and libraries.
This test would have to be constantly updated according to the frameworks updates.

[wellwelwel]

To reproduce the error in NextJS, I just created a new app from the latest version and imported mysql2 into ./src/app/page.tsx. Then I performed a connection and executed a query in the same file, inside Home():

---

• npx create-next-app@latest

  • What is your project named? › my-app
  • Would you like to use TypeScript with this project? › No / Yes
  • Would you like to use ESLint with this project? › No / Yes
  • Would you like to use Tailwind CSS with this project? › No / Yes
  • Would you like to use src/ directory with this project? › No / Yes
  • Use App Router (recommended)? › No / Yes
  • Would you like to customize the default import alias? › No / Yes

• cd my-app

• npm i mysql2

• Into ./src/app/page.tsx:

  • import mysql2 from 'mysql2/promise';
    
    export default async function Home() {
       const db = await mysql2.createConnection({
          host: '',
          user: '',
          password: '',
          database: '',
       });
    
       const [rows] = await db.execute('SELECT * FROM `table`');
       console.log(rows);
    
       return <main></main>;
    }

• npm run build

---

Then:

TypeError: LRU is not a constructor

---

You can see this PR working on it implementing this PR changes in ./node_module/mysql2, deleting the .next folder and running the build again.

[sidorares]

I'm thinking to merge the PR since it solves existing pain while not adding any other problems

We can solve 2 issues I mentioned ( communicating compatible note versions, potentially as a major semver release; lru-cache@9 upgrade ) later

[sidorares]

As for the test - I was hoping to see something that can be done at CI time, not a manual repro steps. Ideally with a small number of dependencies. For example, a local webpack config, a script that creates a bundle ( known to expose the original issue ) and then runs a bundle. Again, we can tackle that later

[wellwelwel]

@sidorares, thank you so far!

I will be focusing on my projects for now.

Feel free to reach out if you need anything 🙋🏻‍♂️

[sidorares]

Thanks a lot for your help @wellwelwel 🤛 !