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

Fix a couple of issues with the web build, caused by CLI changes #81

Merged
merged 2 commits into from
Apr 12, 2024
Merged

Fix a couple of issues with the web build, caused by CLI changes #81

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d20be49
Stop including PathCommandProvider (and so node-pty) in web build
AtkinsSJ Apr 12, 2024
8ededf6
Guard use of `process` with checks that we're running on Node
AtkinsSJ Apr 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/ansi-shell/ANSIShell.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ export class ANSIShell extends EventTarget {
}

async doPromptIteration() {
if ( globalThis.force_eot ) {
if ( globalThis.force_eot && this.ctx.platform.name === 'node' ) {
process.exit(0);
}
const { readline } = this.ctx.externs;
Expand Down
6 changes: 4 additions & 2 deletions src/ansi-shell/pipeline/Pipeline.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,10 @@ export class PreparedCommand {
// but for some reason Node crashes first, unless we set this handler,
// EVEN IF IT DOES NOTHING. I also can't find a place to safely remove it,
// so apologies if it makes debugging promises harder.
const rejectionCatcher = (reason, promise) => {};
process.on('unhandledRejection', rejectionCatcher);
if (ctx.platform.name === 'node') {
const rejectionCatcher = (reason, promise) => { };
process.on('unhandledRejection', rejectionCatcher);
}

let exit_code = 0;
try {
Expand Down
2 changes: 1 addition & 1 deletion src/ansi-shell/readline/readline.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ const ReadlineProcessorBuilder = builder => builder
externs.out.write('^C\n');
// Exit if input line is empty
// FIXME: Check for 'process' is so we only do this on Node. How should we handle exiting in Puter terminal?
if ( process && ctx.vars.result.length === 0 ) {
if ( typeof process !== 'undefined' && ctx.vars.result.length === 0 ) {
process.exit(1);
return;
}
Expand Down
10 changes: 7 additions & 3 deletions src/puter-shell/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,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';
Expand Down Expand Up @@ -83,10 +82,15 @@ export const launchPuterShell = async (ctx) => {
await sdkv2.setAPIOrigin(source_without_trailing_slash);
}

// PathCommandProvider is only compatible with node.js for now
// HACK: The import path is split to fool rollup into not including it.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The fact that this works is hilarious, and now I wonder if webpack or any of the other bundlers do actual AST parsing to handle this (or perhaps fail the same way).

Although, for the moment... 👍 this is a feature rather than a bug - thanks Rollup!

const { PathCommandProvider } = (ctx.platform.name === 'node')
? await import('./providers/' + 'PathCommandProvider.js')
: { PathCommandProvider: null };

const commandProvider = new CompositeCommandProvider([
new BuiltinCommandProvider(),
// PathCommandProvider is only compatible with node.js for now
...(ctx.platform.name === 'node' ? [new PathCommandProvider()] : []),
...(PathCommandProvider ? [new PathCommandProvider()] : []),
new ScriptCommandProvider(),
]);

Expand Down