megahertz / electron-log

Simple logging module Electron/Node.js/NW.js application. No dependencies. No complicated configuration.
MIT License
1.27k stars 124 forks source link

__electronLog is not injected #379

Closed xuewenG closed 6 months ago

xuewenG commented 6 months ago

If win uses the session created in the callback of app.on("ready"), there is no __electronLog injected.

import { app, BrowserWindow, session } from "electron";
import logger from "electron-log";

logger.initialize({ preload: true });

const appReadyCallback = () => {
  const PARTITION = "persist:test";

  session.fromPartition(PARTITION);

  const win = new BrowserWindow({
    width: 700,
    height: 700,
    webPreferences: {
      partition: PARTITION,
    },
  });

  win.loadURL("https://github.com");
};

app.on("ready", appReadyCallback);  // __electronLog is not defined
// app.whenReady().then(appReadyCallback); // works fine

Possible reason: The callbacks registered by app.on('ready') are always executed before the callbacks registered by app.whenReady().

app.whenReady().then(() => {
  console.log("whenReady 1");
});

app.once("ready", () => {
  console.log("once ready 1");
});

app.on("ready", () => {
  console.log("on ready 1");
});

app.whenReady().then(() => {
  console.log("whenReady 2");
});

app.on("ready", () => {
  console.log("on ready 2");
});

app.once("ready", () => {
  console.log("once ready 2");
});

Output:

once ready 1
on ready 1
on ready 2
once ready 2
whenReady 1
whenReady 2
xuewenG commented 6 months ago

try to fix: #380

megahertz commented 6 months ago

@xuewenG This lib uses the default session to inject the preload script. That's the primary reason why it doesn't work for your case.

https://github.com/megahertz/electron-log/blob/master/src/main/electronApi.js#L103

I'm going to add a new sessions param to the initialize function for such a case this week.

xuewenG commented 6 months ago

@megahertz I think adding a new sessions param makes the initialize method more complicated. This is because the user has to collect the created sessions via the session-create event before calling the initialize method, and the collection is already done in the setPreloadFileForSessions method.

If the user tries to get already created sessions via session.fromPartition or session.fromPath, he must distinguish whether app.on('ready') or app.whenReady() was used when calling the initialize method. Otherwise it is possible to accidentally create the session early when calling initialize and not pass the options param needed to create the session.

megahertz commented 6 months ago

I agree that passing sessions or partition names to initialize isn't very convenient. But this library should consider both cases: when it's called before a new session is created and after that. Especially in the ESM environment, when imported modules can contain session initialization as a side effect and execution order isn't determined. Your PR is helpful for the case when a new session is created after initialize(). And it would be nice to find a way to use already created sessions.

It's possible to find all initialized sessions through webContents.getAllWebContents().map((w) =>w.session). But there's a chance a user has already created a new session and hasn't initialized a new BrowserWindow yet. Maybe you have a better idea for that case?

xuewenG commented 6 months ago

Yes, if there was a way to get all the created sessions, all problems would be solved. But I checked Electron's documentation again and didn't find an available API.

xuewenG commented 6 months ago

Please just ignore the PR update, there are no code updates.