Npm package #598
Npm package #598
Conversation
…ckaging + npm3, fix various url-related bugs
…t traverse into node_modules when finding workspace files
| @@ -0,0 +1,53 @@ | |||
| #!/usr/bin/env node | |||
There was a problem hiding this comment.
Is this going to go sideways on ubuntu because it's nodejs and not node
There was a problem hiding this comment.
this probably doesn't matter since everything else makes this assumption though
|
|
||
|
|
||
| var browser = !argv["server"]; | ||
| var port = process.env.PORT || argv["port"] || 8080; |
|
|
||
| var browser = !argv["server"]; | ||
| var port = process.env.PORT || argv["port"] || 8080; | ||
| var filepath = argv["_"][0]; // @FIXME: This should really be the first positional argument, not the first of any argument. (undefined if a user flags first). |
There was a problem hiding this comment.
this doesn't seem correct when using flags, need to guard against this
| let base = dep.split("/").pop(); | ||
| console.log("COPYING", dep); | ||
| console.log("NM", fs.readdirSync("node_modules")); |
| @@ -272,10 +274,20 @@ export class EveClient { | |||
| browser.init(data.code); | |||
| } | |||
| if(this.showIDE) { | |||
| // Ensure the URL bar is in sync with the server. | |||
| // @FIXME: This back and forth of control over where we are | |||
| // is an Escherian nightmare. | |||
There was a problem hiding this comment.
It's worth looking into this only being a client message so that the logic is localized and we don't have all the scary back and forth.
There was a problem hiding this comment.
let's turn this into issue to track
| // @FIXME: I think this is vestigial, but need to confirm this. | ||
| // if(documentId.indexOf("/examples/") === -1) { | ||
| // url = `${location.pathname}#/examples/${documentId}`; | ||
| // } |
| let path = "/" + location.hash.split('?')[0].split("#/")[1]; | ||
| console.log("PATH", path, location.hash); | ||
| if(path) { |
There was a problem hiding this comment.
because of the "/" above, this will never be false
| response.end(packaged); | ||
| }); | ||
|
|
||
| app.get("/assets/*", handleStatic); |
There was a problem hiding this comment.
did we check to make sure this works?
| class SocketRuntimeClient extends RuntimeClient { | ||
| socket: WebSocket; | ||
|
|
||
| constructor(socket:WebSocket, withIDE = true) { | ||
| constructor(socket:WebSocket, inEditor:boolean) { |
There was a problem hiding this comment.
since withIDE is used on the client, let's be consistent
…trolOwner, fixing saving external documents to disk
… examples globally
|
@ibdknox please do a follow-up review when you have a second, but I believe this is ready to go. |
| let path = "/" + location.hash.split('?')[0].split("#/")[1]; | ||
| if(path) { | ||
| let path = location.hash && location.hash.split('?')[0].split("#/")[1]; | ||
| if(path && path.length > 2) { |
There was a problem hiding this comment.
In order to be remotely valid it needs to contain at least a character and a slash. Honestly it should be 3, since all files must at least be /
There was a problem hiding this comment.
probably worth putting a comment in or maybe a regex or something?
There's still work to be done here, but we should be at the point now where the missing features are features we haven't had yet. Namely, the editor can't really work in external projects just yet, and multiple workspaces aren't supported in the UI.
Outstanding issues:
Blocking
Non-Blocking
Existing CLI filepath logic doesn't work unless filepath comes before flags
Need a
eveSource.findto find a URI in any workspace. https://github.com/witheve/Eve/blob/npm-package/src/runtime/server.ts#L184Multiple workspaces support missing from the UI
Should move play.witheve to run off the tgz generated by npm rather than a separate packaging script.
Maintenance