tilemap: UMD module build does not have a requirejs export
Currently the UMD module that is built is not really a complete UMD module. it will work without issue in the browser, but I see no “require” at all that would make it compatible with “CommonJS” and so it’s possibly not loaded properly in Electron.
https://www.davidbcalhoun.com/2014/what-is-amd-commonjs-and-umd/
(function (root, factory) {
if (typeof define === 'function' && define.amd) {
// AMD
define(['jquery'], factory);
} else if (typeof exports === 'object') {
// Node, CommonJS-like
module.exports = factory(require('jquery'));
} else {
// Browser globals (root is window)
root.returnExports = factory(root.jQuery);
}
}(this, function ($) {
// methods
function myFunc(){};
// exposed public method
return myFunc;
}));
This is needed for pixi-tilemap to work in gdevelop https://github.com/4ian/GDevelop/pull/1901
About this issue
- Original URL
- State: open
- Created 4 years ago
- Comments: 26 (15 by maintainers)
All done, please try again.
UMD link is correct. Require and ESM builds are in npm.
Looking at the rollup config, I can see the UMD output of rollup if configured as a “Immediately Invoked Function Expression”, which would work for the browser with globals but is not a UMD module: https://github.com/pixijs/pixi-tilemap/blob/4c9dbdb65b71fc26390ef3311e07a628a9b0044b/rollup.config.js#L93-L97
Is this expected? 😃
Changing this to “umd” and removing the
footer
seems to properly generate the UMD canonical code:Unrelated: @blurymind please note that for GDevelop I’ll need to modify the loader code to ensure all the
require
s (require('@pixi/display'), require('@pixi/core'), require('@pixi/constants')
) are working. For now, we only allowrequire("pixi.js")
andrequire("pixi.js-legacy")
to be used. I’ll probably have to alias all the@pixi/...
.Thanks! 👍 I can still see 2
PIXI.CanvasRenderer
, and 1PIXI.utils.createIndicesForQuads
, should they be replaced too?Ah I see! Forget about it then.
Yeah that seems fair and way less complicated than my
global
stuff. Let’s go ahead with this, I don’t see any reason why it would not work with this extra guard.I guess we should also surely remove what is done in exporter.ts? It’s assigning pixi_tilemap to PIXI.tilemap, but the generated code seems not to done anything good:
The issue being that:
PIXI
,Seems that this will be taken care of by the updated assign suggested by @SukantPal?
Then hopefully we’re good in having a well encapsulated, non leaking, but-still-working-in-the-browser-and-augmenting-PIXI-global-in-this-case module 🤞🤞
@4ian The whole point of importing from
@pixi/
modules is so that we don’t have a dependency on thepixi.js
bundle.I think a better solution would be to check if
pixi_tilemap
exists (which means it is in a browser):Note that
this.PIXI.tilemap
is ensured to be defined in the beginning of the file.Sorry I edited my message after you answered 😃 This line is problematic because it’s not working when requiring this module as CommonJS: pixi_tilemap is not defined in this context, because we’re outside the function embedding the library.
I understand that plugins will register themselves into PIXI, here in
PIXI.tilemap
, but currently this is erroring because the global variablepixi_tilemap
is not defined when you run this module in CommonJS (or in AMD from what i’ve seen). Only the path for the browser will work (because the objectglobal.pixi_tilemap = {}
is created then passed to the function embedding the library as the first parameter, calledexports
).Seems like there is still an extra line at the end of the UMD module (problematic because pixi_tilemap is not defined in this context, because we’re outside the function embedding the library): https://github.com/pixijs/pixi-tilemap/blob/2b7fb150cc867da88bc1631d163f71c9b3b54190/dist/pixi-tilemap.umd.js#L961
Apart from that, seems like a working module 😃
Ah I see, I was assuming Pixi.js was UMD/CommonJS as it can be properly
require
d when installed from npm.This being said, I see that now the umd output seems to be properly a UMD module: https://github.com/pixijs/pixi-tilemap/blob/master/dist/pixi-tilemap.umd.js since your commit https://github.com/pixijs/pixi-tilemap/commit/9c9c1c73424b27bb2b5b053d6fe0208037ba2c20 to use
@pixi-build-tools/rollup-configurator
😃 Some seems we’re now good?The output is not “totally orthodox” because things are still assigned to global PIXI and PIXI.Tilemap objects, but I guess it’s no problem for us (apart from polluting a bit the globals in Electron, it should still work, and should still work in the browser). Time to give this another try I think @blurymind 😃 (We might have another issue pending but unrelated to the build I think)