@aduth I tried to not change so much... but it was hard. The only part that doesn't seem to be working is the benchmark script. Let me know what you think.

Edit: I change it to"type": "module", it seemed simpler, and the declaration file was output with an m in it when made from an .mjs file.

@aduth this is in a better state for review.

Would it make the changes any easier if the package were ported fully to ES Modules, and/or if the pre-built UMD file were removed? These would be breaking changes, but I'd be open to them.

I moved it to ES Modules, and did remove the UMD builds. If that's not the direction you want to go, please let me know and I can revert it. I don't know exactly the purpose the the call to process.env I added the replace plugin for the es build, but left it out on the cjs build. please let me know what the appropriate settings would be.

I suspect ESLint may not be testing this file, since I'd expect semicolons.

I'm having trouble figuring out why ESLint isn't flagging semicolons. Could you take a look at it and see how your custom ESLint config is being applied and what we need to do to fix the issue?

Additionally

• I walked back some package updates to see if it fixed the ESLint issue, but it didn't. I could update everything and chase down the issues. This would be easier for me than to figuring out the config of old versions of the dependencies.

We could probably remove the NODE_ENV=production part of the build:bundle script in package.json as well, since it was related to this.

We still need to remove the process.env otherwise it will cause an error in the browser for accessing a property on the undefined process variable.

Was the issue because it's using the non-TypeScript version of the rule? I thought I'd seen that in an earlier commit. If it's a case of the TypeScript version working better, that sounds fine to enable the TypeScript version if the code passes with it. (The common ESLint configuration should probably be doing that by default)

The TypeScript version didn't make a difference, it was part of how I was trying to track down the issue.

I don't know if the rules of your default config are being applied, when I add "semi": "error" to the rules, it then errors on missing semicolons.

While testing this out with in Gutenberg this error happened. Changing unknown to any fixed it (...args: any[]) => any.

packages/core-data/src/hooks/use-query-select.ts:101:34 - error TS2345: Argument of type 'EnrichedSelectors' is not assignable to parameter of type '(...args: unknown[]) => unknown'.
  Types of parameters 'selectors' and 'args' are incompatible.
    Type 'unknown' is not assignable to type 'Record<string, (...args: any[]) => any>'.

101 const enrichSelectors = memoize( ( ( selectors ) => {
                                     ~~~~~~~~~~~~~~~~~~~~
102  const resolvers = {};
    ~~~~~~~~~~~~~~~~~~~~~~
... 
140  return resolvers;
    ~~~~~~~~~~~~~~~~~~
141 } ) as EnrichedSelectors );
    ~~~~~~~~~~~~~~~~~~~~~~~~

FYI: It was also good to know the original issue I was trying to solve is fixed.

@johnhooks Hopefully you don't mind me pushing directly to your branch. I just pushed up some commits. Would you mind taking a look over them and letting me know if you'd have any objections or if I overstepped at all?

A few remarks:

• I think we probably could move toward the non-ES5 version of the ESLint configuration, but there's a lingering question of what's a reasonable baseline support for syntax that I'm not really sure how to address. I think changing the ecmaVersion to support ESM and leaving the rest alone for now may help reduce the scope.
  • I think this also makes it safe to avoid Babel, since the source syntax is already targeting broad support, and we are no longer compiling for CommonJS
• I like the idea of introducing Prettier, but it'll have a pretty widespread impact on the source, more than I'd want to chew off here. I think the only thing that had been missing from what you added was the eslint-plugin-prettier and eslint-config-prettier dependencies that the shared ESLint configuration keys off of
  • This is probably why semicolons aren't being enforced, since the shared configuration probably doesn't bother much with styling, and assumes we'd enable Prettier if we cared about it.
• With "type": "module" we should be able to avoid the .mjs changes
• Good point about needing to keep the NODE_ENV replacement, since it's not guaranteed that a consuming project would be replacing that in its browser builds

Thanks for another great pull request! I'll try to get this published shortly.

Thank you @aduth, always a pleasure!

This is published now as memize@2.0.0 🚀

That's awesome!