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

v2.0.5 is a breaking change for CommonJS users

Open MichaelBuhler opened this issue 3 years ago • 9 comments

I think the code in v2.0.5 is a breaking change for CommonJS users, such as my organization.

The underlying cause is because of the quick-lru dependency that was added is ESM only.

I guess more broadly, anyone who is requireing geotiff in a CommonJS package with see a failure with [email protected] because Node cannot require the quick-lru module in geotiff.js/dist-module/source/blockedsource.js around line 7. They will see an error message, such as:

/path/to/code/node_modules/geotiff/dist-node/source/blockedsource.js:7
const quick_lru_1 = __importDefault(require("quick-lru"));
                                    ^

Error [ERR_REQUIRE_ESM]: require() of ES Module /path/to/code/node_modules/quick-lru/index.js from /path/to/code/node_modules/geotiff/dist-node/source/blockedsource.js not supported.
Instead change the require of index.js in /path/to/code/node_modules/geotiff/dist-node/source/blockedsource.js to a dynamic import() which is available in all CommonJS modules.
    at Object.<anonymous> (/path/to/code/node_modules/geotiff/dist-node/source/blockedsource.js:7:37)
    at Object.<anonymous> (/path/to/code/node_modules/geotiff/dist-node/source/remote.js:6:28)
    at Object.<anonymous> (/path/to/code/node_modules/geotiff/dist-node/geotiff.js:33:21)
    ... {
  code: 'ERR_REQUIRE_ESM'
}

Not sure how to proceed here, maybe a warning should be added to release notes or something.

Also (I don't have a lot of knowledge in this area), is there any advantage to building and distributing the dist-node folder if you have to use ESM imports now?

Another option is to downgrade quick-lru to v5.1.1 as the ESM breaking change is in v6.0.0. See here.

Refactoring my whole app to "type": "module" probably can't happen soon, so I will miss out on some good features slated for the next release (v2.0.6 perhaps).

MichaelBuhler avatar Apr 13 '22 17:04 MichaelBuhler

I ran into this issue as well. I use geotiff as a nested dependency in geomask. It's far from a workaround for general use, but I got around this issue by basically returning an empty object from require when it hits an ESM Import error. I'm providing the solution here in case it is enough for your use case:

see https://github.com/DanielJDufour/geomask/blob/main/require-esm-as-empty-object.js:

const Module = require("module");

const __require__ = Module.prototype.require;

Module.prototype.require = function () {
  try {
    return __require__.apply(this, arguments);
  } catch (error) {
    if (error.message.includes("require() of ES Module")) {
      return {};
    } else {
      throw error;
    }
  }
};

I too am hoping for a more general solution because I use geotiffjs in tens of small libraries and converting everything to ES6 would be a pain (as well as potentially causing issues for other users).

fwiw, I've had some success in the past converting ESM syntax to CJS syntax doing string replaces (replacing export default with module.exports = , but it's rather brittle.

Sorry I can't be more helpful, but I'll try to brainstorm other solutions, too.

DanielJDufour avatar Apr 14 '22 23:04 DanielJDufour

I published the code above in its own npm module (https://github.com/DanielJDufour/require-esm-as-empty-object), so you can do: node -r require-esm-as-empty-object app.js

DanielJDufour avatar Apr 16 '22 15:04 DanielJDufour

Did you try importing geotiff using a dynamic import as the error suggests? Not sure if this handles converting all underlying dependencies too. Something like the following you might be able to put into your own wrapper module:

const geotiff = await import('geotiff');
export default geotiff

(Edit) Hopefully you read it as an inquisitive question, not accusatory 😄 I'm actually curious if this will work. I've read that dynamic import is "the" bailout for CJS modules to be able to use esm modules. And I'm not sure what its limits are.

twelch avatar May 17 '22 03:05 twelch

@twelch , that's a great suggestion. I haven't tried it yet, but that would be a awesome :-)

fwiw, I'm running into the same error when I'm running ts-node.

DanielJDufour avatar May 21 '22 17:05 DanielJDufour

@DanielJDufour @MichaelBuhler here's some quick tests in node16 and ts-node that seem to work. Note the jsFail script that reproduces the error, and index.js is successful - https://github.com/twelch/geotiff-dynamic-import-test

For ts-node I had to do a little something extra - https://github.com/TypeStrong/ts-node/discussions/1290.

twelch avatar May 22 '22 17:05 twelch

I like @twelch 's solutions, but if you want a quick bandaid to get things working in test (but not production), I published require-esm-deasync, which you can use to get quick-lru required synchronously. I use geotiff.js in a lot of the setup parts of test scripts and didn't want to refactor more than updating a node test.js to node -r require-esm-deasync test.js

DanielJDufour avatar Aug 29 '22 01:08 DanielJDufour

I'm using it here: https://github.com/GeoTIFF/geotiff-precise-bbox/blob/main/package.json#L18

    "test": "node -r require-esm-deasync ./test.js",

DanielJDufour avatar Aug 29 '22 01:08 DanielJDufour

I am unable to get any of these solutions to work for me.

ashmortar avatar Mar 27 '24 22:03 ashmortar

so has anyone solved this question

zhanghjtx avatar Apr 18 '24 01:04 zhanghjtx