-
Notifications
You must be signed in to change notification settings - Fork 260
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
v3: compiled CLI via pkg not working #543
Comments
No ESM support in pkg yet:
Checking how to resolve that. |
Got it working, as CommonJS compiled with rollup and as static binary with pkg:
Minimal changes that I did: Diffdiff -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 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):
|
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}`); node-export-server/lib/browser.js Lines 21 to 27 in c8a360a
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. |
@cvasseng can this logic be removed again and a new v3 patch release done? |
@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. |
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): 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. |
Install pkg:
Compile CLI with NodeJS 20:
Log
Run compiled CLI:
So currently there are a few problems, which should be resolved to make the CLI compilable.
The text was updated successfully, but these errors were encountered: