search

reg-viz / storycap

A Storybook Addon, Save the screenshot image of your stories :camera: via puppeteer.
https://www.npmjs.com/package/storycap
MIT License
701 stars 89 forks source link

The linux() function in find-chrome.ts doesn't check $CHROME_PATH, but an error message incorrectly suggests setting $CHROME_PATH in certain situations. #840

Open nerikeshi-k opened 6 months ago

nerikeshi-k commented 6 months ago

Thank you for this great project!

Problem I encountered

Environment:

The Storycap CLI command I ran:

$ storycap --serverCmd=\"npx serve dist-storybook -p 6006\" http://localhost:6006"

The error message I received:

> storycap --serverCmd="npx serve dist-storybook -p 6006" http://localhost:6006 "--outDir=.vrt/expected"

info Wait for connecting storybook server http://localhost:6006.
error The environment variable CHROME_PATH must be set to executable of a build of Chromium version 54.0 or later.
Error: The environment variable CHROME_PATH must be set to executable of a build of Chromium version 54.0 or later.
    at linux (/home/neriko/misc/vrt-artifacts/node_modules/.pnpm/storycrawler@4.2.0/node_modules/storycrawler/lib/find-chrome.js:126:15)
    at findChrome (/home/neriko/misc/vrt-artifacts/node_modules/.pnpm/storycrawler@4.2.0/node_modules/storycrawler/lib/find-chrome.js:163:30)
    at StoriesBrowser.boot (/home/neriko/misc/vrt-artifacts/node_modules/.pnpm/storycrawler@4.2.0/node_modules/storycrawler/lib/browser/base-browser.js:47:71)
    at main (/home/neriko/misc/vrt-artifacts/node_modules/.pnpm/storycap@4.2.0/node_modules/storycap/lib/node/main.js:45:101)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async time (/home/neriko/misc/vrt-artifacts/node_modules/.pnpm/storycrawler@4.2.0/node_modules/storycrawler/lib/timer.js:14:20)

I attempted to set the environment variable CHROME_PATH as suggested by the error message but it didn't solve the issue.

I suspect the function to find the Chromium binary on Linux, named linux(), might be partly incorrect. It seems to use $CHROME_PATH only for calculating priority, but does not search within $CHROME_PATH. linux() is here: https://github.com/reg-viz/storycap/blob/7c222313ced3dcc04196cc791a5ffdaf1fafe3df/packages/storycrawler/src/find-chrome.ts#L103-L148

Suggestions

I see two possible solutions:

  1. Remove the code related to $CHROME_PATH as its current use might be causing unintended behaviors. Here is an example:

    /**
* Look for linux executables in 2 ways
* 1. Look into the directories where .desktop are saved on gnome based distro's
* 2. Look for google-chrome-stable & google-chrome executables by using the which command
*/
function linux(_canary = false): string | undefined {
let installations: string[] = [];

// Look into the directories where .desktop are saved on gnome based distro's
const desktopInstallationFolders = [
  path.join(require('os').homedir(), '.local/share/applications/'),
  '/usr/share/applications/',
];
desktopInstallationFolders.forEach(folder => {
  installations = installations.concat(findChromeExecutables(folder));
});

// Look for google-chrome(-stable) & chromium(-browser) executables by using the which command
const executables = ['google-chrome-stable', 'google-chrome', 'chromium-browser', 'chromium'];
executables.forEach(executable => {
  try {
    const chromePath = execFileSync('which', [executable], { stdio: 'pipe' }).toString().split(newLineRegex)[0];
    if (canAccess(chromePath)) installations.push(chromePath);
  } catch (e) {
    // Not installed.
  }
});

const priorities = [
  { regex: /chrome-wrapper$/, weight: 51 },
  { regex: /google-chrome-stable$/, weight: 50 },
  { regex: /google-chrome$/, weight: 49 },
  { regex: /chromium-browser$/, weight: 48 },
  { regex: /chromium$/, weight: 47 },
];

return sort(uniq(installations.filter(Boolean)), priorities)[0] ?? undefined;
}

  2. Make the function search within $CHROME_PATH. Here's an example:

    /**
* Look for linux executables in 3 ways
* 1. Look into CHROME_PATH env variable
* 2. Look into the directories where .desktop are saved on gnome based distro's
* 3. Look for google-chrome-stable & google-chrome executables by using the which command
*/
function linux(_canary = false) {
let installations: string[] = [];

// Add CHROME_PATH if defined
if (process.env.CHROME_PATH)
  installations.push(process.env.CHROME_PATH);

// Look into the directories where .desktop are saved on gnome based distro's
const desktopInstallationFolders = [
  path.join(require('os').homedir(), '.local/share/applications/'),
  '/usr/share/applications/',
];
desktopInstallationFolders.forEach(folder => {
  installations = installations.concat(findChromeExecutables(folder));
});

// Look for google-chrome(-stable) & chromium(-browser) executables by using the which command
const executables = ['google-chrome-stable', 'google-chrome', 'chromium-browser', 'chromium'];
executables.forEach(executable => {
  try {
    const chromePath = execFileSync('which', [executable], { stdio: 'pipe' }).toString().split(newLineRegex)[0];
    if (canAccess(chromePath)) installations.push(chromePath);
  } catch (e) {
    // Not installed.
  }
});

if (!installations.length)
  throw new Error(
    'The environment variable CHROME_PATH must be set to executable of a build of Chromium version 54.0 or later.',
  );

const priorities = [
  { regex: /chrome-wrapper$/, weight: 51 },
  { regex: /google-chrome-stable$/, weight: 50 },
  { regex: /google-chrome$/, weight: 49 },
  { regex: /chromium-browser$/, weight: 48 },
  { regex: /chromium$/, weight: 47 },
];

if (process.env.CHROME_PATH) priorities.unshift({ regex: new RegExp(`${process.env.CHROME_PATH}`), weight: 101 });

return sort(uniq(installations.filter(Boolean)), priorities)[0];
}

If either way 1 or 2 looks ok, I'm going to create a pull request. Thank you!