Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v2.0.5 is a breaking change for CommonJS users #301

Open
MichaelBuhler opened this issue Apr 13, 2022 · 9 comments
Open

v2.0.5 is a breaking change for CommonJS users #301

MichaelBuhler opened this issue Apr 13, 2022 · 9 comments

Comments

@MichaelBuhler
Copy link
Contributor

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 geotiff@2.0.5 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).

@DanielJDufour
Copy link
Contributor

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
Copy link
Contributor

DanielJDufour commented Apr 16, 2022

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

@twelch
Copy link

twelch commented May 17, 2022

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.

@DanielJDufour
Copy link
Contributor

@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.

@twelch
Copy link

twelch commented May 22, 2022

@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 - TypeStrong/ts-node#1290.

@DanielJDufour
Copy link
Contributor

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
Copy link
Contributor

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",

@ashmortar
Copy link

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

@zhanghjtx
Copy link

so has anyone solved this question

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants