highcharts / node-export-server

Highcharts Node.js export server
Other
354 stars 260 forks source link

v3: compiled CLI via pkg not working #543

Open DanielRuf opened 2 months ago

DanielRuf commented 2 months ago

Install pkg:

npm install -g @yao-pkg/pkg

Compile CLI with NodeJS 20:

Log ``` pkg . -t node20-linux-x64,node20-macos-x64,node20-win-x64 > pkg@5.12.0 > Fetching base Node.js binaries to PKG_CACHE_PATH fetched-v20.11.1-linux-x64 [====================] 100% fetched-v20.11.1-macos-x64 [====================] 100% fetched-v20.11.1-win-x64 [====================] 100% > Warning Cannot include directory %1 into executable. The directory must be distributed with executable as %2. %1: ../puppeteer/.local-chromium %2: path-to-executable/puppeteer > Warning Cannot include directory %1 into executable. The directory must be distributed with executable as %2. %1: ../puppeteer/.local-chromium %2: path-to-executable/puppeteer > Warning Babel parse has failed: import.meta may appear only with 'sourceType: "module"' (24:55) > Warning Babel parse has failed: import.meta may appear only with 'sourceType: "module"' (70:49) > Warning Babel parse has failed: import.meta may appear only with 'sourceType: "module"' (27:49) > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/bin/cli.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/index.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/config.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/utils.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/server/server.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/chart.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/logger.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/pool.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/cache.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/schemas/config.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/server/rate_limit.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/server/routes/health.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/server/routes/export.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/server/routes/change_hc_version.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/server/routes/ui.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/browser.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/export.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/fetch.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/bin/cli.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/index.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/config.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/utils.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/server/server.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/chart.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/logger.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/pool.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/cache.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/schemas/config.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/server/rate_limit.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/server/routes/health.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/server/routes/export.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/server/routes/change_hc_version.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/server/routes/ui.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/browser.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/export.js > Warning Failed to make bytecode node20-x64 for file /snapshot/node_modules/highcharts-export-server/lib/fetch.js > Warning Failed to make bytecode node20-x64 for file C:\snapshot\node_modules\highcharts-export-server\bin\cli.js > Warning Failed to make bytecode node20-x64 for file C:\snapshot\node_modules\highcharts-export-server\lib\index.js > Warning Failed to make bytecode node20-x64 for file C:\snapshot\node_modules\highcharts-export-server\lib\config.js > Warning Failed to make bytecode node20-x64 for file C:\snapshot\node_modules\highcharts-export-server\lib\utils.js > Warning Failed to make bytecode node20-x64 for file C:\snapshot\node_modules\highcharts-export-server\lib\server\server.js > Warning Failed to make bytecode node20-x64 for file C:\snapshot\node_modules\highcharts-export-server\lib\chart.js > Warning Failed to make bytecode node20-x64 for file C:\snapshot\node_modules\highcharts-export-server\lib\logger.js > Warning Failed to make bytecode node20-x64 for file C:\snapshot\node_modules\highcharts-export-server\lib\pool.js > Warning Failed to make bytecode node20-x64 for file C:\snapshot\node_modules\highcharts-export-server\lib\cache.js > Warning Failed to make bytecode node20-x64 for file C:\snapshot\node_modules\highcharts-export-server\lib\schemas\config.js > Warning Failed to make bytecode node20-x64 for file C:\snapshot\node_modules\highcharts-export-server\lib\server\rate_limit.js > Warning Failed to make bytecode node20-x64 for file C:\snapshot\node_modules\highcharts-export-server\lib\server\routes\health.js > Warning Failed to make bytecode node20-x64 for file C:\snapshot\node_modules\highcharts-export-server\lib\server\routes\export.js > Warning Failed to make bytecode node20-x64 for file C:\snapshot\node_modules\highcharts-export-server\lib\server\routes\change_hc_version.js > Warning Failed to make bytecode node20-x64 for file C:\snapshot\node_modules\highcharts-export-server\lib\server\routes\ui.js > Warning Failed to make bytecode node20-x64 for file C:\snapshot\node_modules\highcharts-export-server\lib\browser.js > Warning Failed to make bytecode node20-x64 for file C:\snapshot\node_modules\highcharts-export-server\lib\export.js > Warning Failed to make bytecode node20-x64 for file C:\snapshot\node_modules\highcharts-export-server\lib\fetch.js ```

Run compiled CLI:

./highcharts-export-server-linux 
node:internal/modules/cjs/loader:1432
      throw err;
      ^

Error [ERR_REQUIRE_ESM]: require() of ES Module /snapshot/node_modules/highcharts-export-server/bin/cli.js not supported.
Instead change the require of cli.js in null to a dynamic import() which is available in all CommonJS modules.
    at Function.runMain (pkg/prelude/bootstrap.js:1983:12) {
  code: 'ERR_REQUIRE_ESM'
}

So currently there are a few problems, which should be resolved to make the CLI compilable.

DanielRuf commented 2 months ago

No ESM support in pkg yet:

Checking how to resolve that.

DanielRuf commented 2 months ago

Got it working, as CommonJS compiled with rollup and as static binary with pkg:

npm run build
pkg .
node cli.cjs --enableServer 1
./highcharts-export-server-linux --enableServer 1

Minimal changes that I did:

Diff ```diff diff -ur ./lib/browser.js ./lib/browser.js --- ./lib/browser.js 2024-07-20 12:47:35.360839085 +0000 +++ ./lib/browser.js 2024-07-20 15:22:45.108441533 +0000 @@ -16,7 +16,7 @@ import fs from 'fs'; import * as url from 'url'; import { log } from './logger.js'; -import path from 'node:path'; +import path from 'path'; // Workaround for https://bugs.chromium.org/p/chromium/issues/detail?id=1463328 // Not ideal - leaves trash in the FS @@ -67,10 +67,8 @@ '--use-mock-keychain' ]; -const __dirname = url.fileURLToPath(new URL('.', import.meta.url)); - const template = fs.readFileSync( - __dirname + '/../templates/template.html', + path.join(process.cwd(), 'templates/template.html'), 'utf8' ); @@ -78,7 +76,7 @@ const setPageContent = async (page) => { await page.setContent(template); - await page.addScriptTag({ path: __dirname + '/../.cache/sources.js' }); + await page.addScriptTag({ path: path.join(process.cwd(), '.cache/sources.js') }); // eslint-disable-next-line no-undef await page.evaluate(() => window.setupHighcharts()); diff -ur ./lib/cache.js ./lib/cache.js --- ./lib/cache.js 2024-07-20 12:47:35.364839084 +0000 +++ ./lib/cache.js 2024-07-20 15:23:10.420440453 +0000 @@ -17,18 +17,17 @@ // before starting the service import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'fs'; -import { join } from 'path'; +import path from 'path'; import dotenv from 'dotenv'; import HttpsProxyAgent from 'https-proxy-agent'; import { fetch } from './fetch.js'; import { log } from './logger.js'; -import { __dirname } from '../lib/utils.js'; dotenv.config(); -const cachePath = join(__dirname, '.cache'); +const cachePath = path.join(process.cwd(), '.cache'); const cache = { cdnURL: 'https://code.highcharts.com/', @@ -71,7 +70,7 @@ try { writeFileSync( - join(cachePath, 'manifest.json'), + path.join(cachePath, 'manifest.json'), JSON.stringify(newManifest), 'utf8' ); @@ -207,8 +206,8 @@ export const checkCache = async (config) => { let fetchedModules; // Prepare paths to manifest and sources from the .cache folder - const manifestPath = join(cachePath, 'manifest.json'); - const sourcePath = join(cachePath, 'sources.js'); + const manifestPath = path.join(cachePath, 'manifest.json'); + const sourcePath = path.join(cachePath, 'sources.js'); // TODO: deal with trying to switch to the running version // const activeVersion = appliedConfig ? appliedConfig.version : false; diff -ur ./lib/export.js ./lib/export.js --- ./lib/export.js 2024-07-20 12:47:35.452839081 +0000 +++ ./lib/export.js 2024-07-20 15:28:04.624427889 +0000 @@ -24,8 +24,6 @@ import path from 'path'; import * as url from 'url'; -const __basedir = url.fileURLToPath(new URL('.', import.meta.url)); - // const jsonTemplate = require('./../templates/json_export/json_export.js'); /** @@ -301,7 +299,7 @@ } else if (options.customCode.allowFileResources) { injectedResources.push( await page.addStyleTag({ - path: path.join(__basedir, cssImportPath) + path: path.join('.', cssImportPath) }) ); } diff -ur ./lib/server/routes/health.js ./lib/server/routes/health.js --- ./lib/server/routes/health.js 2024-07-20 12:47:35.460839080 +0000 +++ ./lib/server/routes/health.js 2024-07-20 14:48:11.112530099 +0000 @@ -14,13 +14,13 @@ import cache from '../../cache.js'; import pool from '../../pool.js'; +import path from 'path'; import { readFileSync } from 'fs'; -import { __dirname } from './../../utils.js'; -import { join as pather } from 'path'; const pkgFile = JSON.parse( - readFileSync(pather(__dirname, 'package.json')) + // hint: use __dirname when bundling asssets with pkg, otherwise use process.cwd() + readFileSync(path.join(__dirname, 'package.json')) ); const serverStartTime = new Date(); diff -ur ./lib/server/routes/ui.js ./lib/server/routes/ui.js --- ./lib/server/routes/ui.js 2024-07-20 12:47:35.536839077 +0000 +++ ./lib/server/routes/ui.js 2024-07-20 15:00:44.048497946 +0000 @@ -12,9 +12,8 @@ *******************************************************************************/ -import { join } from 'path'; +import path from 'path'; -import { __dirname } from '../../utils.js'; /** * Adds the / route for a UI when enabled for the export server */ @@ -22,5 +21,5 @@ !app ? false : app.get('/', (request, response) => { - response.sendFile(join(__dirname, 'public', 'index.html')); + response.sendFile(path.join(process.cwd(), 'public/index.html')); }); diff -ur ./lib/server/server.js ./lib/server/server.js --- ./lib/server/server.js 2024-07-20 12:47:35.524839078 +0000 +++ ./lib/server/server.js 2024-07-20 15:24:28.736437108 +0000 @@ -24,7 +24,6 @@ import { log } from '../logger.js'; import rateLimit from './rate_limit.js'; -import { __dirname } from '../utils.js'; import healthRoute from './routes/health.js'; import exportRoutes from './routes/export.js'; @@ -162,7 +161,7 @@ } // Set up static folder's route - app.use(express.static(posix.join(__dirname, 'public'))); + app.use(express.static(posix.join(process.cwd(), 'public'))); // Set up routes healthRoute(app); diff -ur ./lib/utils.js ./lib/utils.js --- ./lib/utils.js 2024-07-20 12:47:35.536839077 +0000 +++ ./lib/utils.js 2024-07-20 14:47:55.524530764 +0000 @@ -13,16 +13,13 @@ *******************************************************************************/ import { readFileSync } from 'fs'; -import { fileURLToPath } from 'url'; -import { join as pather } from 'path'; +import path from 'path'; import { defaultConfig } from '../lib/schemas/config.js'; import { log } from './logger.js'; const MAX_BACKOFF_ATTEMPTS = 6; -export const __dirname = fileURLToPath(new URL('../.', import.meta.url)); - /** * Clears text from whitespaces with a regex rule. * @@ -282,7 +279,7 @@ export const printLogo = (noLogo) => { // Get package version either from env or from package.json const packageVersion = JSON.parse( - readFileSync(pather(__dirname, 'package.json')) + readFileSync(path.join(__dirname, 'package.json')) ).version; // Print text only @@ -293,7 +290,7 @@ // Print the logo console.log( - readFileSync(__dirname + '/msg/startup.msg').toString().bold.yellow, + readFileSync(path.join(__dirname, 'msg/startup.msg')).toString().bold.yellow, `v${packageVersion}` ); }; diff -ur ./package.json ./package.json --- ./package.json 2024-07-20 12:47:35.588839075 +0000 +++ ./package.json 2024-07-20 15:10:14.672473579 +0000 @@ -16,7 +16,7 @@ "type": "git" }, "bin": { - "highcharts-export-server": "./bin/cli.js" + "highcharts-export-server": "./cli.cjs" }, "scripts": { "install": "node install.js", @@ -29,7 +29,6 @@ "http-tests-single": "node tests/http/http_test_runner_single.js", "node-tests": "node tests/node/node_test_runner.js", "node-tests-single": "node tests/node/node_test_runner_single.js", - "prepare": "husky install", "build": "rollup -c" }, "devDependencies": { @@ -43,6 +42,14 @@ "prettier": "^3.0.0", "rollup": "^3.29.4" }, + "pkg": { + "assets": [ "public/" ], + "targets": [ + "node20-linux-x64", + "node20-macos-x64", + "node20-win-x64" + ] + }, "dependencies": { "body-parser": "^1.20.2", "colors": "1.4.0", diff -ur ./rollup.config.js ./rollup.config.js --- ./rollup.config.js 2024-07-20 12:47:35.520839078 +0000 +++ ./rollup.config.js 2024-07-20 13:57:27.212660081 +0000 @@ -3,18 +3,13 @@ // exporting the rollup config export default { - input: 'lib/index.js', + input: 'bin/cli.js', output: [ { - file: 'dist/index.esm.js', - format: 'es', - sourcemap: true - }, - { - file: 'dist/index.cjs', + file: 'cli.cjs', format: 'cjs', - sourcemap: 'inline' + sourcemap: false } ], - plugins: [terser()] + plugins: [] }; ```

Minimal testing via --enableServer 1 was successful. I think it makes sense to either publish binaries directly (we do not want to install and run NodeJS on the server) or at least document the way to create a binary with pkg.

Also I think there should be some of my changes in the highcharts-export-server code, as they may make the code more portable.

The patch / diff can be applied like this (alternatively use patch-package or yarn / pnpm):

wget https://registry.npmjs.org/highcharts-export-server/-/highcharts-export-server-3.1.1.tgz
tar xzf highcharts-export-server-3.1.1.tgz
patch --directory=package/ --strip=1 < patch.diff
DanielRuf commented 2 months ago

The following code should probably not be used then, because it fills the tmp folder with every execution.

const RANDOM_PID = node_crypto.randomBytes(64).toString('base64url');
const PUPPETEER_DIR = path.join('tmp', `puppeteer-${RANDOM_PID}`);

https://github.com/highcharts/node-export-server/blob/c8a360ab8e8bdf5cb7f6566040b5afeab0589a74/lib/browser.js#L21-L27

I guess this needs some further improvement.

Looks like in v4 there is not this logic with the mentioned code anymore: https://github.com/highcharts/node-export-server/blob/e86e39c52b65d4ffe940ec56e47b438091b0bb0f/lib/browser.js

Regarding statically compiled binaries (can be added to GitHub releases), these would make usage without NodeJS on servers much easier.

DanielRuf commented 2 months ago

@cvasseng can this logic be removed again and a new v3 patch release done?

jszuminski commented 2 months ago

@DanielRuf I've analyzed this issue now and in v4.0.0 we got rid of this part of problematic logic that you've mentioned (as you've correctly pointed out). Thus, this is no longer a problem on the current master branch (which contains the v4.0.0 that is about to be published on npm).

The v4.0.0 includes drastic performance improvements and multiple bug fixes. It has officially been deployed as the Highcharts Export Server (https://export.highcharts.com/) with a success rate of over 99%.

I recommend you try it out. We will release it on npm soon, once we fully update the changelog.


Regarding the second part of your question which is the prebuilt binary, I have added it to our backlog so that we can discuss it in the future. After all the prioritized issues are solved (bugs & performance always have the highest priority), we may include this in one of the upcoming releases so I'm leaving this issue open.

DanielRuf commented 2 months ago

I would like to use the CLI of v4 as statically compiled binary.

Because this way we can get rid of NodeJS as separate dependency on servers in production.

Currently our setup looks like that (on Windows): PHP => bat file => node export.js => highcharts-export-server

My goal is this:

PHP => highcharts-export-server CLI (statically compiled)

I will try to adjust the patch for v4 and clean it up, so that I can get rid of the overly complex solution (which leads to multiple issues because of that).

My second best solution currently is to use npx, which would be PHP => npx => highcharts-export-server CLI.