Skip to content
This repository has been archived by the owner on Apr 13, 2024. It is now read-only.

node-pty dependency breaks running Phoenix in browser #78

Open
AtkinsSJ opened this issue Apr 11, 2024 · 0 comments
Open

node-pty dependency breaks running Phoenix in browser #78

AtkinsSJ opened this issue Apr 11, 2024 · 0 comments

Comments

@AtkinsSJ
Copy link
Contributor

Running Phoenix in the browser now fails because of the node-pty dependency added for the PathCommandProvider. This causes us to include a binary file, which the rollup common-js plugin gets very upset about.

We don't need or want that included, since PathCommandProvider only works when running in the Node CLI. However, excluding source files from rollup is less straightforward than I'd hoped and I'm not sure what I'm doing. 😅

We can prevent the code including that with this change:

diff --git a/src/puter-shell/main.js b/src/puter-shell/main.js
index 9c2715c..3898be2 100644
--- a/src/puter-shell/main.js
+++ b/src/puter-shell/main.js
@@ -27,7 +27,6 @@ import { Context } from "contextlink";
 import { SHELL_VERSIONS } from "../meta/versions.js";
 import { PuterShellParser } from "../ansi-shell/parsing/PuterShellParser.js";
 import { BuiltinCommandProvider } from "./providers/BuiltinCommandProvider.js";
-import { PathCommandProvider } from "./providers/PathCommandProvider.js";
 import { CreateChatHistoryPlugin } from './plugins/ChatHistoryPlugin.js';
 import { Pipe } from '../ansi-shell/pipeline/Pipe.js';
 import { Coupler } from '../ansi-shell/pipeline/Coupler.js';
@@ -36,6 +35,12 @@ import { MultiWriter } from '../ansi-shell/ioutil/MultiWriter.js';
 import { CompositeCommandProvider } from './providers/CompositeCommandProvider.js';
 import { ScriptCommandProvider } from './providers/ScriptCommandProvider.js';
 
+// PathCommandProvider is only compatible with node.js for now
+let PathCommandProvider;
+if (typeof window === 'undefined') {
+    PathCommandProvider = await import('./providers/PathCommandProvider.js');
+}
+
 const argparser_registry = {
     [SimpleArgParser.name]: SimpleArgParser
 };

...however, rollup can't tell that this (or whatever other check) doesn't include PathCommandProvider.js, so it adds it anyway.

A temporary workaround is to not include it at all:

diff --git a/src/puter-shell/main.js b/src/puter-shell/main.js
index 9c2715c..9cb6d9b 100644
--- a/src/puter-shell/main.js
+++ b/src/puter-shell/main.js
@@ -27,7 +27,6 @@ import { Context } from "contextlink";
 import { SHELL_VERSIONS } from "../meta/versions.js";
 import { PuterShellParser } from "../ansi-shell/parsing/PuterShellParser.js";
 import { BuiltinCommandProvider } from "./providers/BuiltinCommandProvider.js";
-import { PathCommandProvider } from "./providers/PathCommandProvider.js";
 import { CreateChatHistoryPlugin } from './plugins/ChatHistoryPlugin.js';
 import { Pipe } from '../ansi-shell/pipeline/Pipe.js';
 import { Coupler } from '../ansi-shell/pipeline/Coupler.js';
@@ -36,6 +35,9 @@ import { MultiWriter } from '../ansi-shell/ioutil/MultiWriter.js';
 import { CompositeCommandProvider } from './providers/CompositeCommandProvider.js';
 import { ScriptCommandProvider } from './providers/ScriptCommandProvider.js';
 
+// PathCommandProvider is only compatible with node.js for now
+let PathCommandProvider;
+
 const argparser_registry = {
     [SimpleArgParser.name]: SimpleArgParser
 };
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant