enonic / xp

Enonic XP
https://enonic.com
GNU General Public License v3.0
198 stars 34 forks source link

Global namespace #6775

Open ComLock opened 5 years ago

ComLock commented 5 years ago

It would be ideal if the global namespace where as similar as possible within the browser, node and Enonic XP environments.

As such I think we should make a libApp to provide name, version and config properties (or functions). Then deprecate (with a log.warning) the use of the global app object in Enonic XP 7.0 And then remove it entirely in Enonic XP 8.

The same probably goes for most the other things we have added to the global namespace.

ref: https://xp.readthedocs.io/en/stable/developer/ssjs/global-objects.html

Hopefully the global or window object becomes available in Enonic XP 7. Not to share things between controllers, but to make sure as many as possible node modules work.

ref: https://javascript.info/global-object

ComLock commented 5 years ago

Here is an example of why adding things to the global namespace is a bad idea: https://github.com/enonic/lib-util/blob/master/src/main/resources/site/lib/enonic/util/index.es#L23

If we need a global logger I think we should use console which already exists in the browser and node.

sigdestad commented 5 years ago

This is interesting.. Need to discuss the details!

sigdestad commented 5 years ago

@aro what is being used in PurpleJS?

ComLock commented 5 years ago

Example node modules that doesn't work in Enonic XP 6.x:

//import parse from 'csv-parse'; // global.Buffer.TYPED_ARRAY_SUPPORT undefined
//import parse from 'csv-parse/lib/sync'; // global.Buffer.TYPED_ARRAY_SUPPORT undefined
//import parse from 'csv-string'; // global.Buffer.TYPED_ARRAY_SUPPORT undefined
//import csv from 'csvtojson'; // setTimeout is not defined
//import csvdata from 'csvdata'; // Uses fs
//import csv from 'csv-parser'; // global.Buffer.TYPED_ARRAY_SUPPORT undefined
ComLock commented 5 years ago

Webpack 4.26.1 breaks transpilation of modules that depend on the global object for us. So use webpack 4.25.1

In browser context:

In node context

ref: https://github.com/webpack/webpack/compare/v4.25.1...v4.26.1#diff-a5b27cd0606ad3e895b1e11e729e9c14R10

ComLock commented 5 years ago

There is actually a a global object in Enonic XP (nashorn?) It can be fetched by doing an indirect call to eval in order to get variables from the global scope.

log.info((1, eval)(“this”)); --> [object global] log.info(JSON.stringify((1, eval)(“this”))); --> {}

So this in global context is an empty object.

https://stackoverflow.com/questions/9107240/1-evalthis-vs-evalthis-in-javascript

ComLock commented 5 years ago

Webpack workaround https://github.com/ComLock/enonic-xp-app-crawler/commit/32b140ffd97bfe93d59fc18e6fdbd3728c19bba8

sigdestad commented 5 years ago

Good ideas indeed. We should aim to implement this before we release 8.0