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

Error: require ESM modules with relative paths #613

Closed
indus opened this issue Dec 8, 2023 · 10 comments
Closed

Error: require ESM modules with relative paths #613

indus opened this issue Dec 8, 2023 · 10 comments

Comments

@indus
Copy link

indus commented Dec 8, 2023

I've just updated to the latest version and I faced problems using require on a local script.

mapshaper -require "./myESM.mjs"
mapshaper -require "./myCommon.js"

both result in an Error: Only URLs with a scheme in: file, data, and node are supported by the default ESM loader. On Windows, absolute paths must be valid file:// URLs. Received protocol 'c:'

only the following works:
mapshaper -require "file:///C:/myProj/myESM.mjs"
mapshaper -require "file:///C:/myProj/myCommon.js"

I think the problem is here https://github.com/mbloch/mapshaper/blob/master/src/commands/mapshaper-require.mjs#L24
It changes the relative to an absolute path but misses to add the required file:///

I'm working on Windows (in case that makes a difference). I've testet Node 18 and 20 LTS

@indus
Copy link
Author

indus commented Dec 8, 2023

I've just noticed that even with an valid absolute path to my script I get really strange errors:

with the script poly.mjs

export const polygon = function () {
    return {
        type: "FeatureCollection",
        features: [{
            type: "Feature",
            geometry: { type: "Point", coordinates: [125.6, 10.1] },
            properties: { name: "Dinagat Islands" }
        }]
    }
}
mapshaper `
-rectangle bbox="-180,-90,180,90" `
-require "file:///C:/myProj/poly.mjs" `
-run "target.geojson = polygon()"

Error: [i] File not found (target.geojson)

mapshaper `
-rectangle bbox="-180,-90,180,90" `
-require "file:///C:/myProj/poly.mjs" `
-run "polygon()"

Error: [i] File not found (polygon())

@mbloch
Copy link
Owner

mbloch commented Dec 8, 2023

Thanks for reporting this... the problem was evidently caused by my switching from require() to import(). This was in order to support ES modules in addition to Node modules. The new code worked fine on my Mac, but I see that import() fails on Windows using absolute paths, and according to the Node docs import() is not supposed to work with absolute paths. This issue clarifies things: nodejs/node#31710

I'll work on a fix asap.

@mbloch
Copy link
Owner

mbloch commented Dec 8, 2023

I published the fix given in the above-linked issue (using pathToFileURL() to convert absolute paths to file URLs). Please let me know if you still get errors -- I don't currently have a way to test on Windows.

@indus
Copy link
Author

indus commented Dec 8, 2023

I've just tested 0.6.58 but had no luck with relative paths:
mapshaper -require "./myScript.js"

[require] Unable to load external module: Cannot find module 'C:\Users\...\AppData\Roaming\nvm\v18.17.1\node_modules\mapshaper\myScript.js' ...

It now looks in the installation folder of the global mapshaper installation...

@mbloch
Copy link
Owner

mbloch commented Dec 8, 2023

Hmm.. I'm going to install Parallels so I can run Windows and test this properly... if that fails I might go back to using require()

@mbloch
Copy link
Owner

mbloch commented Dec 8, 2023

I haven't been able to reproduce the Unable to load external module error yet... I tested with mapshaper 0.6.58 on Windows 11 (running in Parallels) using both Node 18 and Node 20 in cmd.exe. -require works in 0.6.58 and throws the error you first reported in 0.6.57. Here's a screengrab:
image

Do you have any ideas that might help me reproduce your error? Are you using a different shell than cmd.exe?

@mbloch
Copy link
Owner

mbloch commented Dec 8, 2023

-require works in powershell too, in my current setup:
image

@indus
Copy link
Author

indus commented Dec 8, 2023

🤦‍♂️ I'm so sorry wasting your time on this. My paths changed after I moved the scripts around.
I only realized after I tested it on a different laptop. So I guess only the initial error was valid.
Again - Sorry.

@mbloch
Copy link
Owner

mbloch commented Dec 8, 2023

Well, at least I got my Windows test environment set up ;)

I also published another version without the following lines, which I thought might have been responsible for the problem:

if (moduleFile && !require('path').isAbsolute(moduleFile)) {
moduleFile = require('path').join(process.cwd(), moduleFile);
}

(mapshaper was converting relative paths to absolute paths, because require() didn't work correctly with some relative paths).

import() seems to work correctly with various forms of relative paths, so it's no longer necessary to convert to absolute paths.

@indus
Copy link
Author

indus commented Dec 8, 2023

I'm glad you were able to find something positive in it.
But I would prefer to close the issue in shame 😄

@indus indus closed this as completed Dec 8, 2023
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

2 participants