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

Make cli and standalone binaries build work #407

Merged
merged 2 commits into from
Sep 18, 2024

Conversation

ahocevar
Copy link
Contributor

@ahocevar ahocevar commented Sep 17, 2024

Fixes #398.

I failed to run the cli on MacOS with node20:

$ npx geostyler-cli -s sld -t mapbox --output out.json in.sld
Need to install the following packages:
[email protected]
Ok to proceed? (y) y

/Users/ahocevar/.npm/_npx/d2b78a7bedb6eda9/node_modules/geostyler-cli/dist/src/index.js:17
const geostyler_qgis_parser_1 = __importDefault(require("geostyler-qgis-parser"));
                                                ^

Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/ahocevar/.npm/_npx/d2b78a7bedb6eda9/node_modules/geostyler-qgis-parser/dist/QGISStyleParser.js from /Users/ahocevar/.npm/_npx/d2b78a7bedb6eda9/node_modules/geostyler-cli/dist/src/index.js not supported.
Instead change the require of QGISStyleParser.js in /Users/ahocevar/.npm/_npx/d2b78a7bedb6eda9/node_modules/geostyler-cli/dist/src/index.js to a dynamic import() which is available in all CommonJS modules.
    at Object.<anonymous> (/Users/ahocevar/.npm/_npx/d2b78a7bedb6eda9/node_modules/geostyler-cli/dist/src/index.js:17:49) {
  code: 'ERR_REQUIRE_ESM'
}

Node.js v20.17.0

Also, building the binaries spits out hundreds of babel and bytecode warnings. It seems pkg does not pick up the bin setting from package.json properly

This pull request fixes both problems:

  • A single geostyler.js file without dependencies is a good entry point for both pkg and running the cli bin
  • Inlining the pkg config in the pkg cli call and removing the config from package.json fixes the pkg build

The single geostyler.js cli bin has another advantage: all dependencies are now devDependencies, so npx geostyler-cli will install much faster - without the need to download dependency packages.

@ahocevar ahocevar force-pushed the pkg-macos branch 4 times, most recently from cbb7a1f to d0c5ce8 Compare September 17, 2024 20:34
@ahocevar ahocevar mentioned this pull request Sep 17, 2024
@ahocevar ahocevar changed the title Make cli and build work on MacOS Make cli and standalone binaries build work Sep 17, 2024
@ahocevar ahocevar mentioned this pull request Sep 17, 2024
@KaiVolland
Copy link
Contributor

KaiVolland commented Sep 18, 2024

Would be nice if you could check if this also works with the latests versions of the geostyler packages:

    "geostyler-lyrx-parser": "^1.0.1",
    "geostyler-mapbox-parser": "^6.0.0",
    "geostyler-mapfile-parser": "^4.0.1",
    "geostyler-qgis-parser": "^3.0.0",
    "geostyler-sld-parser": "^6.1.2",
    "geostyler-style": "^9.1.0",

@ahocevar
Copy link
Contributor Author

ahocevar commented Sep 18, 2024

@KaiVolland It does. I pushed a commit which updates those dependencies to the versions you posted above.

@KaiVolland KaiVolland merged commit 8816109 into geostyler:main Sep 18, 2024
3 checks passed
@ahocevar ahocevar deleted the pkg-macos branch September 18, 2024 15:26
@ahocevar
Copy link
Contributor Author

ahocevar commented Oct 4, 2024

@KaiVolland Any chance you could publish a new version of the CLI with the recent fixes? Thanks in advance.

@jansule
Copy link
Contributor

jansule commented Oct 7, 2024

🎉 This PR is included in version 4.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@KaiVolland
Copy link
Contributor

@ahocevar sry that somehow got lost. 👼 🙂

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

Successfully merging this pull request may close these issues.

ERR_REQUIRE_ESM
3 participants