Implement `console.Console` constructor.

DavidIsa commented 1 year ago

What version of Bun is running?

0.6.13

What platform is your computer?

Linux 5.15.49-linuxkit-pr aarch64 aarch64

What steps can reproduce the bug?

Execute this code in bun

const log = new console.Console();

What is the expected behavior?

Should not fail to call new console.Console() and must create a new Console instances.

What do you see instead?

const log = new console.Console();
                ^
TypeError: undefined is not a constructor (evaluating 'new console.Console')

Additional information

This method is implemented in nodejs and some npm packages use it which causes it to fail when using it with bun.

Method documentation for nodejs https://nodejs.org/api/console.html#new-consoleoptions

adsellor commented 1 year ago

If you don't mind, I would like to work on this issue. cc: @paperdave

paperdave commented 1 year ago

go for it

i would add this in ConsoleObject.ts

basically implement it in js using Bun.inspect

make it similar to how the async iterator works in terms of wiring it up to the console object.

adsellor commented 1 year ago

Great. Thanks for the tips. I'll get started on this

joaogl commented 1 year ago

Hey @adsellor, I'm also facing this issue while trying to start my nestjs project. How's the progress on this? is there anyway I can help?

adsellor commented 1 year ago

Hey @joaogl. Sorry for taking this long and not updating my findings here. Here is what I found while trying to make this work.

In NodeJS the Console constructor accepts either (stdout, stderr) parameters, or a single options parameter.
The console itself is an instance of Console constructor where stdout and stderr are passed are set to process.stdout and process.stderr.
In Bunjs, there is (ZigConsoleClient)[https://github.com/oven-sh/bun/blob/main/src/bun.js/bindings/exports.zig#L921], which is initialized (here)[https://github.com/oven-sh/bun/blob/main/src/bun.js/javascript.zig#L937] and assigned to the global VM.

I wasn't able to do it ConsoleObject.ts, as @paperdave suggested, as it requires initialization of ZigConsoleClient itself (Maybe I just missed something and it can be done). But essentially, what I was trying to do, is assigning Console to the ZigConsoleClient and returning a new instance, and extending the client itself to support all the options that Node does (like inspectOptions and colorMode). However, we can't pass Writer as a parameter with callconv(.C). I've also tried implementing it in ZigConsoleClient.cpp, but I don't think that extending it with a non-JSC default is a good idea. A lot of things got into my way last week and I didn't really have time to explore other options, so if you are down to help, your input would be much appreciated.

joaogl commented 1 year ago

Hey @adsellor, don't expect too much from me :D first time looking into a codebase like this one.

Is there any documentation around the codebase structure or architecture besides what is in https://bun.sh/docs/project/development that I could use to warm up?

adsellor commented 1 year ago

Hey @adsellor, don't expect too much from me :D first time looking into a codebase like this one.

Is there any documentation around the codebase structure or architecture besides what is in https://bun.sh/docs/project/development that I could use to warm up?

I don't think so, and I don't feel confident enough to give an advice on where to start :D . But maybe join discord and ask questions along the way.

P.S I'll get back to this during this weekend, hoping to have at least a simple Console constructor with stdout working.

paperdave commented 1 year ago

Is there any documentation around the codebase structure or architecture besides what is in https://bun.sh/docs/project/development that I could use to warm up?

unfortunatly there isnt. but if you're in our discord i can answer any questions about the project. in #dev-general there.

this documentation page really needs updating.

paperdave commented 1 year ago

also, the idea i was thinking was to use Bun.inspect* or util.inspect to format the arguments to a string, then write the string to the given node stream.

I dont think we need to do much of this in Zig/C++. I don't really imagine this feature is used too heavily that it needs to be extremely fast either.

Also it's stdout and stderr, not stdin. I was confused for a second before checking docs.

*the note about Bun.inspect is right now it actually does almost exactly what we need, but we want to change the signature of this function from inspect(...): string to inspect(value, options): string to be a little closer to node's util inspect (and be more useful overall). But in this case it's probably worth keeping the version of inspect that works like a console formatter, maybe we'll do that only as a builtin function.

adsellor commented 1 year ago

Also it's stdout and stderr, not stdin. I was confused for a second before checking docs.

Sorry for the confusion, it should've been stderr instead of stdin, my mind was somewhere else. I've updated the comment.

*the note about Bun.inspect is right now it actually does almost exactly what we need, but we want to change the signature of this function from inspect(...): string to inspect(value, options): string to be a little closer to node's util inspect (and be more useful overall). But in this case it's probably worth keeping the version of inspect that works like a console formatter, maybe we'll do that only as a builtin function.

I see, I'll try to implement it this way. Thanks for the advice.

joaogl commented 1 year ago

@adsellor if you don't mind, please share the PR once you start looking into this, I'm deeply interested in understand how this will turn out and to look into the changes you make to learn something 🙏

webivan1 commented 1 year ago

I was able to solve this issue in docker with using image node:latest and installing bun with command: npm install -g bun after that I haven't seen any issue in my console

FROM node:latest

WORKDIR /app

RUN npm install -g bun

COPY package*.json ./
COPY bun.lockb bun.lockb

RUN bun install

COPY . .

RUN bun run build

EXPOSE 3000

CMD ["./docker_resources/entrypoint.sh"]

Electroid commented 1 year ago

@webivan1 Thanks for sharing, that might work for your particular script, but we'll still need to implement console.Console.

webivan1 commented 1 year ago

@Electroid Sorry, I forgot to mention It was for nestjs framework I had same error console.Console when I use docker image oven/bun

oven-sh / bun