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

-require local script file vs. npm package #646

Open
indus opened this issue Jun 23, 2024 · 4 comments
Open

-require local script file vs. npm package #646

indus opened this issue Jun 23, 2024 · 4 comments

Comments

@indus
Copy link

indus commented Jun 23, 2024

I'm struggling with -require (again ;-)

I'm having a bunch of scripts that extend mapshaper functionality that I use from time to time. To make it easier to use them on other systems I wanted to put them in a registry so I would be able to load them with npm. I was surprised that this makes quite a difference.

Some of my findings:

  • with a global mapshaper install the my addon scripts also need to be installed globally (thats ok - but I was a bit surprised)
  • With local script files I was able to call the functions without curly braces - with a npm package this is no longer possible as the functions are no longer directly added to the global context but below the package name (or alias).
  • I tried to add them to the global context exporting a function but had no luck (besides it sounds deprecated)
  • the scripts no longer have access to the context via this when called with -run. This gave a lot of flexibility.

There are probably good reasons for those differences between those two types of "-require" but maybe you could give me a hint what the best-practice is to create a plugin-library that combines multiple commands - that is easy to distribute, load and call...

@mbloch
Copy link
Owner

mbloch commented Jun 23, 2024

Hi!

I should be able to make some changes to support your use cases. Part of the problem may be that mapshaper is using import() instead of require() to load external modules. import() can load node-style modules, but (as far as I understand) doesn't search the same set of paths as require(), which explains why modules installed locally with npm aren't loading. I could try using require() if import() fails.

I'll look into the global context issues you reported.

@indus
Copy link
Author

indus commented Jun 23, 2024

I investigated a bit further and i think the last bullet (no access to context) is just caused because the function is enforced to be a nested property of the alias and not on the global level.

with this script...

module.exports = {
    fnA:function(){
        console.log(Object.getOwnPropertyDescriptors(this))
    },
    nested:{
        fnB:function(){
            console.log(Object.getOwnPropertyDescriptors(this))
        }
    }
}
mapshaper -require "./index.js" -run "{fnA()}" -run "{nested.fnB()}"

... fnA has context in this and nested.fnB does not

At the moment functions in npm modules are always a nested properties. This make the difference?!

@indus
Copy link
Author

indus commented Jun 23, 2024

Just to give you some context. The functions I use most often generate geojson output based on geojson input. And the template I use looks something like this:

const fnName = "myFn"

module.exports = {
    [fnName]: function (arg1, arg2, arg3) {
        const { $, target } = this;

        if ($) {
            const geojson = $.geojson;

            // generate modified geojson

            return geojsonMOD;

        } else if (target) {

            const fnName_ = `[+ ${fnName}]`;

            console.info(`${fnName_} The target gets cloned and renamed - use '-each ${fnName}(...)' to alter it in place`)

            let args = [...arguments].map(JSON.stringify)

            let name = [target.layer_name, `_${fnName}`, ...args]

            return `-filter "true" target=${target.layer_name} + name=${name.join('_')}
            -each this.geojson=${fnName}(${args.join(',')})`;

        }
    }
}

This allows the function to be called with -run myFn(1,2,3) to create a new layer based on the current target as well as with -each this.geojson = myFn(1,2,3) to change the current target in place with the same arguments.

(maybe I should use the io api of -run to generate new layer instead of copying them with -filter "true"🤔 )

@indus
Copy link
Author

indus commented Jun 24, 2024

I've just tested how to make the context thing work. Turns out that If npm modules would also be asigned to global like so the context would be available with this. This would fix this issue as well.

Maybe the functions in the module should be assigned to global by default and only should nested if alias is used

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