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 little runtime as bin that loads in this dir so npx will work #2

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

konsumer
Copy link

This resolves #1

I just added a simple CLI that uses serve to serve the directory of the project, so if you publish, you can do this, without installing or having git (as long as you have node installed):

npx tilemap-editor

Additionally, you can install it globally and use it:

npm i -g tilemap-editor
tilemap-editor

@@ -26,8 +26,14 @@
"url": "https://github.com/blurymind/tilemap-editor/issues"
},
"homepage": "https://github.com/blurymind/tilemap-editor#readme",
"dependencies": {
"serve-handler": "^6.1.3"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we also make this a dev-dependency?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it needs to be a dependency, for the CLI to work in npx.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One terrible hack might be to combine npx with serve, in the script, instead:

#!/usr/bin/env node

const { spawn } = require('child_process')
process.chdir(__dirname)
const proc = spawn('npx', ["serve"])
proc.stdout.pipe(process.stdout)

You could go further and remove serve from devDependencies and set start to npx serve.

This seems like a dark-pattern though, in order to not have a dependency listed (calling npx, inside a potential npx call) and since the only people who are running this command (vs using the deployed copy, at a URL) should probably install it's deps properly via npm (using the dependencies field) I think the other way is a bit better.

Copy link
Owner

@blurymind blurymind Jul 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we not simply do this
http://www.sheshbabu.com/posts/publishing-npx-command-to-npm/
why even have a serve handler? :) is it because you want to run it via nodejs without installing it anywhere?

I am assuming npx tilemap-editor@latest does that.

I guess the intention here is that you run this command and tiled-editor just starts working on a localhost server? Does that run the index.html file?

Why do you want to do that, when you can simply use the PWA?
https://blurymind.github.io/tilemap-editor/

seems easier to me to visit https://blurymind.github.io/tilemap-editor/ and even install it so it works offline, than to run npx tilemap-editor@latest

If that is the case, there is no justification to add a dependency to it imo

Copy link
Author

@konsumer konsumer Jul 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we not simply do this

if you look at that link, you will see you need to make a javascript handler that does what you want (that you put in the bin field) which is what I did. That handler has serve as a dependency.

I am assuming npx tilemap-editor@latest does that.

No.

seems easier to me to visit https://blurymind.github.io/tilemap-editor/

Yeh, agreed. That is another way to run the editor, if you don't want to run it locally.

and even install it so it works offline, than to run npx tilemap-editor@latest

Not sure what you mean. Try that. Does it do what you expect? Without a handler, it doesn't run anything, so there is no local web-server.

If that is the case, there is no justification to add a dependency to it imo

Well, that's up to you. Seems like a weird choice to me. Like the runtime code has no added dependency, it's just so the npx command works. If you don't want the npx command to work, then don't do it, but that was the point of this PR.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If not having a dependency listed is so important, I offer a solution above. I don't think it's good (listing the dependency is better) but that would resolve the "I don't want a dependency listed"

@blurymind
Copy link
Owner

Thank you for making a pr. Can you also add some instructions to the README as to how to pull it via npx?

@konsumer
Copy link
Author

Ok, done.

If you'd like to install it, globally:

```sh
npm i -g tilemap-editor
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that works even now though, right?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no. did you try it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here is an example of it not working: https://asciinema.org/a/gPMlSSiDh2lNbbB6ahazONJ9B

@zommerfelds
Copy link

A bit unrelated but I was wondering how hard it would be to make this tool be able to automatically load maps and tilesets from a local directory, and to be able to save them via the server instead of manual file download.

My usecase would be to run this on a cloud IDE (e.g. Codespaces) and let you open the editor right in the browser. But since the files are all remote it would be cool if the editor could save files via the web server:

tilemap-editor --map assets/maps.json --tileset assets/tileset.png

Anyway, maybe this is just my personal use case, feel free to ignore. :)

@konsumer
Copy link
Author

konsumer commented Dec 19, 2023

@zommerfelds It's been a couple years since I made this (unmerged) PR. The idea here is it just runs a little web-server, so the tilemap-editor web-app itself would need a way to preload assets, like based on a URL-hash or something.

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

Successfully merging this pull request may close these issues.

npx
3 participants