-
-
Notifications
You must be signed in to change notification settings - Fork 18
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
base: main
Are you sure you want to change the base?
Conversation
@@ -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" |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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"
Thank you for making a pr. Can you also add some instructions to the README as to how to pull it via npx? |
Ok, done. |
If you'd like to install it, globally: | ||
|
||
```sh | ||
npm i -g tilemap-editor |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
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:
Anyway, maybe this is just my personal use case, feel free to ignore. :) |
@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. |
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):
Additionally, you can install it globally and use it: