Mis-detecting Existence of Node.js

[adrianirwin]

The determination of whether Node.js is present or not is made here (line 54, pptxgen.js)

var NODEJS = ( typeof module !== 'undefined' && module.exports );

However, it is not foolproof, and can lead to a false positive, as it will also pass when CommonJS is present. Please review this thread from Stack Overflow for a possible solution.

[adrianirwin]

Apologies in advance if this is a little vague, I'm trying to come up with a workable solution for my current project. Will update this when I can.

[gitbrent]

Hi @adrianirwin,

I understand what you're saying, and i'm not against improving node detection, however, NODEJS only fulfills one main purpose in the library: determining how to save files (it's also used for encoding media).

There's only two ways to save files:

• Node mode: If you can use fs and/or streaming
• Browser mode: save files using window methods: e.g.: window.URL.createObjectURL

The setBrowser flag is used to force browser mode.

Can you use that or are you facing the opposite issue where you need to force node?

[DSheffield]

I ran into a similar problem (mis-detects as node) trying to use pptxgenjs with Ember. Although you mentioned that NodeJS is only used for saving, it still causes problems (i.e. exceptions) in the client code when the lib is imported, due to the require('jquery-node') in the code at the end of pptxgen.js below. Since we're running in the browser there is no jquery-node module.

// [Node.js] support
if ( NODEJS ) {
	// A: Set vars
	var isElectron = require("is-electron");

	// B: Load depdendencies
	var fs = require("fs");
	var $ = isElectron() ? require("jquery") : require("jquery-node");
	var JSZip = require("jszip");
	var sizeOf = require("image-size");

	// C: Export module
	module.exports = PptxGenJS;
}

[gitbrent]

@DSheffield,

Yikes, I didn't consider the potential for clobbering libraries and/or causing exceptions when the library goes to load Node dependencies.

I needed to update Node detection anyway, so i've updated the code to check for fs now, which is basically the point - Node detection determines how the library attempts to save.

Thanks for the feedback. Let me know if the new version works better once the newest NPM version is released.

[DSheffield]

I copied the change you made into my local copy to see how it worked. If I imported it in Ember in one way, it would detect as node, but as browser if I used a diff import method. Ember allows you to import things directly out of your node_modules (e.g. like import _ from 'npm:lodash';). If I did it this way it detected as node and threw an exception on the import due to the line:

var $ = isElectron() ? require("jquery") : require("jquery-node");, and jquery-node not being present.

If I made it globally available as per https://guides.emberjs.com/v2.18.0/addons-and-dependencies/managing-dependencies/#toc_other-assets, however, it correctly detects as browser.

I'm wondering if perhaps we could defer any decisions related to environment (node / browser) as late as possible, so that there is an opportunity to call setBrowser before that happens and then take the appropriate steps. Perhaps this would solve some of the issues people have seen with other frameworks misdetecting?

[gitbrent]

Hi @DSheffield,

I've removed the electron detection library and made the loading of library dependencies more robust using try/catch.

This will fix the error with loading of jquery-node but I'm not sure that addresses all of your issues as there's still no delayed loading of fs etc.

So, you're using this now?
var NODEJS = ( typeof module !== 'undefined' && module.exports && typeof require === 'function' && require('fs') );

If so, do you mind trying with the new changes: Change Commit

[SSS2557]

@gitbrent , I tried the change but it still results in this error when I try to save a presentation from the sample.

Uncaught (in promise) TypeError: fs.writeFile is not a function at pptxgen.js:1687
This means, its still considering it to be run on nodejs.

else { // Starting in late 2017 (Node ~8.9.1), fs requires a callback so use a dummy func zip.generateAsync({type:'nodebuffer'}).then(function(content){ fs.writeFile(strExportName, content, function(){} ); }); }

[gitbrent]

Hi @SSS2557 ,

What version on PptxGenJS are you using?