UMD module build does not have a requirejs export

[blurymind]

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
4ian/GDevelop#1901

[4ian]

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:

function (global, factory) {
    typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('@pixi/display'), require('@pixi/core'), require('@pixi/constants'), require('@pixi/math'), require('@pixi/graphics'), require('@pixi/sprite')) :
    typeof define === 'function' && define.amd ? define(['exports', '@pixi/display', '@pixi/core', '@pixi/constants', '@pixi/math', '@pixi/graphics', '@pixi/sprite'], factory) :
    (global = typeof globalThis !== 'undefined' ? globalThis : global || self, factory(global.pixi_tilemap = {}, global.PIXI, global.PIXI, global.PIXI, global.PIXI, global.PIXI, global.PIXI));
}(this, (function (exports, display, core, constants, math, graphics, sprite) { 'use strict';

Unrelated: @blurymind please note that for GDevelop I'll need to modify the loader code to ensure all the requires (require('@pixi/display'), require('@pixi/core'), require('@pixi/constants')) are working. For now, we only allow require("pixi.js") and require("pixi.js-legacy") to be used. I'll probably have to alias all the @pixi/....

[ShukantPal]

I'm afraid that pixi.js itself doesn't output to UMD, however.

[ShukantPal]

@4ian Why aren't you using CommonJS for Electron instead of UMD?

[4ian]

We are, but I figured out UMD would be convenient as it's for example what is used for pixi-multistyle-text (see the generated file here) and it has the advantage of working:

• In Electron (we "just" require it, and get back MultiStyleText that we can then use to render it in the editor).
• In the browser, in which case it's exported as a global, that we can use directly. This is what we use for the game engine (we don't have any bundling for the game engine).

So I was supposing UMD would be the more convenient way of exporting a module, as it works in CommonJS and as a browser global (and as a AMD require) with a single file :)
Maybe I missed something though that would make impossible/not satisfactory to have this module as UMD?

If really required, we can always embed two files for pixi-tilemap, one as a browser global (used by the game engine), and another one as a CommonJS module (for the editor).

[ShukantPal]

@4ian I would suggest you file an issue at pixijs/pixi.js and ping @bigtimebuddy. He said he was open to it - but nobody asked for this until you. The pixi.js bundle, as of now, is in IIFE format. On Aug 25, 2020, at 1:54 PM, Florian Rival <notifications@github.com<mailto:notifications@github.com>> wrote: We are, but I figured out UMD would be convenient as it's for example what is used for pixi-multistyle-text<https://github.com/tleunen/pixi-multistyle-text> (see the generated file here<https://github.com/4ian/GDevelop/blob/master/Extensions/BBText/pixi-multistyle-text/dist/pixi-multistyle-text.umd.js>) and it has the advantage of working: * In Electron (we "just" require it, and get back MultiStyleText that we can then use to render it in the editor<https://github.com/4ian/GDevelop/blob/082318d7e4e1a8a92a1318dc235f8296f52631e8/Extensions/BBText/JsExtension.js#L479>). * In the browser, in which case it's exported as a global, that we can use directly<https://github.com/4ian/GDevelop/blob/master/Extensions/BBText/bbtextruntimeobject-pixi-renderer.js#L14>. This is what we use for the game engine (we don't have any bundling for the game engine). So I was supposing UMD would be the more convenient way of exporting a module, as it works in CommonJS and as a browser global (and as a AMD require) with a single file :) Maybe I missed something though that would make impossible/not satisfactory to have this module as UMD? If really required, we can always embed two files for pixi-tilemap, one as a browser global (used by the game engine), and another one as a CommonJS module (for the editor). — You are receiving this because you commented. Reply to this email directly, view it on GitHub<#97 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AFLJDB4HUNIBPSP7ELP3CTTSCP3GBANCNFSM4QHB3YAQ>.

[4ian]

Ah I see, I was assuming Pixi.js was UMD/CommonJS as it can be properly required 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 9c9c1c7 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)

[blurymind]

I will give this another try in gdevelop when I get some time :)

[ivanpopelyshev]

All done, please try again.

UMD link is correct.
Require and ESM builds are in npm.

[4ian]

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 :)

[ShukantPal]

I don’t think that line will be removed. All PixiJS plugins are supposed to insert their API into a namespace, which is PIXI.tilemap here. You can see this for all @pixi/, pixi.js, pixi.js-legacy bundles as well. The global for pixi-tilemap isn’t pixi_tilemap, rather is PIXI.tilemap! https://github.com/SukantPal/pixi-build-tools/blob/9942761ab8873bae2ed1d1bcc809addd8194d893/packages/globals/plugins.js#L10 On Aug 29, 2020, at 12:40 PM, Florian Rival <notifications@github.com> wrote: Seems like there is still an extra line at the end of the UMD module: https://github.com/pixijs/pixi-tilemap/blob/2b7fb150cc867da88bc1631d163f71c9b3b54190/dist/pixi-tilemap.umd.js#L961 Apart from that, seems like a working module :) — You are receiving this because you commented. Reply to this email directly, view it on GitHub<#97 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AFLJDB53LSVGA6SJXSSB7A3SDEVOJANCNFSM4QHB3YAQ>.

[4ian]

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 variable pixi_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 object global.pixi_tilemap = {} is created then passed to the function embedding the library as the first parameter, called exports).

[bpkennedy]

Can confirm that the 2.1.1 npm package simply isn't exporting the library correctly. I rolled back to npm version 2.0.6 and it works.

I am not using whatever pixi bundling/rollup is mentioned a couple times elsewhere. I am simply getting npm packages and doing inside a Vue project:

import * as PIXI from 'pixi'
...
window.PIXI = PIXI // this is needed for some reason??
require('pixi-tilemap') // this MUST happen after importing PIXI and assigning to window

Versions:
"pixi-tilemap": "^2.0.6",
"pixi.js": "^5.3.3",

[ShukantPal]

@4ian Should it then be

if (typeof window !== 'undefined') {
  Object.assign(PIXI.tilemap, window.pixi_tilemap);
}

[ShukantPal]

@bpkennedy If you are using import/export, you now simply

import { RectTileLayer } from 'pixi-tilemap'

directly.

[4ian]

There seems to be two problems:

1. The one highlighted by @bpkennedy: a global object called PIXI is still required. This is because there are still references to "PIXI" in the generated UMD module - which seems like mistakes/forgotten references. One example:

In the UMD build:
https://github.com/pixijs/pixi-tilemap/blob/430adfc6f186f1bdaa55cccd7f5ce4a69570dabf/dist/pixi-tilemap.umd.js#L49
this is coming from this source that is indeed referencing PIXI:
https://github.com/pixijs/pixi-tilemap/blob/430adfc6f186f1bdaa55cccd7f5ce4a69570dabf/src/RectTileLayer.ts#L3-L22

The solution is I believe to remove these PIXI.Xyz to replace them by a import { Xyz } from '@pixi/...'; (I'm not super familiar with how is modularized Pixi, but seems that it's working well for the rest).
Should be doable by just searching for PIXI. in the sources.

2. Second problem is that while we're bundling as a UMD, we still want in the case of the browser globals (i.e: when we're not in an AMD environment and not in a CommonJS environment) to define PIXI.tilemap to be what is exported as global.pixi_tilemap.

Seems like this is handled by this code in the rollup configurator but as we've seen, it would be failing in the case of CommonJS/AMD.

I think there are two solutions:

• Get the global PIXI to be a dependency, so that it's passed to the module and we can set PIXI.tilemap = pixi_tilemap, with pixi_tilemap being what is defined here.
  • For this, I guess it's a matter of adding an import * as PIXI from 'pixi.js' in index.ts? Then do PIXI.tilemap = pixi_tilemap. In a way this is what was kind of done in exporter.ts, but I think that what exporter.ts is doing is useless.
• The other solution: rework the Object.assign. Instead of window, I would rather use global:

if (typeof global.PIXI !== 'undefined' && typeof global.pixi_tilemap !== 'undefined') {
  Object.assign(global.PIXI.tilemap, global.pixi_tilemap);
}

but I'm not entirely sure, because playing with global is risky (the generated UMD code goes a great length for global, doing: global = typeof globalThis !== 'undefined' ? globalThis : global || self, which might be to be compatible with web workers).

I would rather try the first solution (import * as PIXI from 'pixi.js' and then assign PIXI.tilemap) first, as it would make a UMD module with "no hacks" (just am assignement to PIXI, which is fine as it's done inside the UMD module).

[ivanpopelyshev]

OK, I removed PIXI.xxx references, 2.1.2 , please try again. I dont understand what do you want about PIXI.tilemap namespace.

Guys, I just remind you that I only slightly understand whats going on :)

[ShukantPal]

@4ian The whole point of importing from @pixi/ modules is so that we don't have a dependency on the pixi.js bundle.

I think a better solution would be to check if pixi_tilemap exists (which means it is in a browser):

if (typeof pixi_tilemap !== 'undefined) {
  Object.assign(this.PIXI.tilemap, pixi_tilemap);
}

Note that this.PIXI.tilemap is ensured to be defined in the beginning of the file.

[4ian]

OK, I removed PIXI.xxx references, 2.1.2 , please try again

Thanks! 👍
I can still see 2 PIXI.CanvasRenderer, and 1 PIXI.utils.createIndicesForQuads, should they be replaced too?

The whole point of importing from @pixi/ modules is so that we don't have a dependency on the pixi.js bundle.

Ah I see! Forget about it then.

I think a better solution would be to check if pixi_tilemap exists (which means it is in a browser):

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:

    var pixi_tilemap;
    (function (pixi_tilemap) {
        PIXI.tilemap = pixi_tilemap;
    })(pixi_tilemap || (pixi_tilemap = {}));
    var exporter = {};

The issue being that:

• it uses the global PIXI,
• it assign something to PIXI.tilemap, but this something is always an empty object.

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 🤞🤞

[ShukantPal]

@4ian We should be done here?

[blurymind]

@ivanpopelyshev @4ian @SukantPal thanks everyone for getting involved :) with your help pixi-tilemap is getting closer to be a part of gdevelop.

I will soon be able to also confirm it works ok on the web version of gdevelop. My ultimate hope is that we dont need to use a patched library in GD or if its patched its minimal

[4ian]

Thanks all!
Though @blurymind could you be able to try the updated library (replace it, then re-run npm start or at least npm run import-resources)? I'm still worried of:

if (typeof pixi_tilemap !== 'undefined) {
  Object.assign(this.PIXI.tilemap, pixi_tilemap);
}

throwing an undefined reference to pixi_tilemap when used in Electron (despite the guard).
If it's working well with the UMD file as is, we can close the issue, just want to double check that.

[spassvogel]

I used to be able to

window.PIXI = PIXI;
import 'pixi-tilemap';

and then
const tileLayer = new window.PIXI.tilemap.CompositeRectTileLayer();

in a typescript project. That doesn't work anymore.

I tried to import { RectTileLayer } from 'pixi-tilemap'. That compiles, but it doesn't render anything :( I'm using 2.1.3.

[ShukantPal]

@spassvogel Can you give us a reproduction? Are there any errors being thrown? Also, be sure to check if pixi-tilemap is not getting its own copy of @pixi/core (or any other pixi package).

[spassvogel]

@spassvogel Are there any errors being thrown?

No errors. just nothing rendered.

Also, be sure to check if pixi-tilemap is not getting its own copy of @pixi/core (or any other pixi package).

How would I see that?

Can you give us a reproduction?

Yes okay I can set up a test project

[ShukantPal]

@spassvogel You can see if pixi-tilemap gets its own copy by: opening node_modules/pixi-tilemap/node_modules and see if there is a @pixi package there.

[spassvogel]

Well I tried to create an isolated case but typescript is complaining about missing typedefs

ERROR in [at-loader] ./node_modules/pixi-tilemap/index.d.ts:1:29 
    TS7016: Could not find a declaration file for module '@pixi/core'. 'repositories/pixi-tilemap-test/node_modules/@pixi/core/lib/core.js' implicitly has an 'any' type.
  Try `npm install @types/pixi__core` if it exists or add a new declaration (.d.ts) file containing `declare module '@pixi/core';`

ERROR in [at-loader] ./node_modules/pixi-tilemap/index.d.ts:2:24 
    TS7016: Could not find a declaration file for module '@pixi/display'. 'repositories/pixi-tilemap-test/node_modules/@pixi/display/lib/display.js' implicitly has an 'any' type.
  Try `npm install @types/pixi__display` if it exists or add a new declaration (.d.ts) file containing `declare module '@pixi/display';`

ERROR in [at-loader] ./node_modules/pixi-tilemap/index.d.ts:3:36 
    TS7016: Could not find a declaration file for module '@pixi/core'. 'repositories/pixi-tilemap-test/node_modules/@pixi/core/lib/core.js' implicitly has an 'any' type.
  Try `npm install @types/pixi__core` if it exists or add a new declaration (.d.ts) file containing `declare module '@pixi/core';`

ERROR in [at-loader] ./node_modules/pixi-tilemap/index.d.ts:4:27 
    TS7016: Could not find a declaration file for module '@pixi/display'. 'repositories/pixi-tilemap-test/node_modules/@pixi/display/lib/display.js' implicitly has an 'any' type.
  Try `npm install @types/pixi__display` if it exists or add a new declaration (.d.ts) file containing `declare module '@pixi/display';`

ERROR in [at-loader] ./node_modules/pixi-tilemap/index.d.ts:5:26 
    TS7016: Could not find a declaration file for module '@pixi/core'. 'repositories/pixi-tilemap-test/node_modules/@pixi/core/lib/core.js' implicitly has an 'any' type.
  Try `npm install @types/pixi__core` if it exists or add a new declaration (.d.ts) file containing `declare module '@pixi/core';`

ERROR in [at-loader] ./node_modules/pixi-tilemap/index.d.ts:6:27 
    TS7016: Could not find a declaration file for module '@pixi/core'. 'repositories/pixi-tilemap-test/node_modules/@pixi/core/lib/core.js' implicitly has an 'any' type.
  Try `npm install @types/pixi__core` if it exists or add a new declaration (.d.ts) file containing `declare module '@pixi/core';`

ERROR in [at-loader] ./node_modules/pixi-tilemap/index.d.ts:7:26 
    TS7016: Could not find a declaration file for module '@pixi/graphics'. 'repositories/pixi-tilemap-test/node_modules/@pixi/graphics/lib/graphics.js' implicitly has an 'any' type.
  Try `npm install @types/pixi__graphics` if it exists or add a new declaration (.d.ts) file containing `declare module '@pixi/graphics';`

ERROR in [at-loader] ./node_modules/pixi-tilemap/index.d.ts:8:24 
    TS7016: Could not find a declaration file for module '@pixi/math'. 'repositories/pixi-tilemap-test/node_modules/@pixi/math/lib/math.js' implicitly has an 'any' type.
  Try `npm install @types/pixi__math` if it exists or add a new declaration (.d.ts) file containing `declare module '@pixi/math';`

ERROR in [at-loader] ./node_modules/pixi-tilemap/index.d.ts:9:32 
    TS7016: Could not find a declaration file for module '@pixi/core'. 'repositories/pixi-tilemap-test/node_modules/@pixi/core/lib/core.js' implicitly has an 'any' type.
  Try `npm install @types/pixi__core` if it exists or add a new declaration (.d.ts) file containing `declare module '@pixi/core';`

ERROR in [at-loader] ./node_modules/pixi-tilemap/index.d.ts:10:27 
    TS7016: Could not find a declaration file for module '@pixi/math'. 'repositories/pixi-tilemap-test/node_modules/@pixi/math/lib/math.js' implicitly has an 'any' type.
  Try `npm install @types/pixi__math` if it exists or add a new declaration (.d.ts) file containing `declare module '@pixi/math';`

ERROR in [at-loader] ./node_modules/pixi-tilemap/index.d.ts:11:26 
    TS7016: Could not find a declaration file for module '@pixi/core'. 'repositories/pixi-tilemap-test/node_modules/@pixi/core/lib/core.js' implicitly has an 'any' type.
  Try `npm install @types/pixi__core` if it exists or add a new declaration (.d.ts) file containing `declare module '@pixi/core';`

ERROR in [at-loader] ./node_modules/pixi-tilemap/index.d.ts:12:27 
    TS7016: Could not find a declaration file for module '@pixi/core'. 'repositories/pixi-tilemap-test/node_modules/@pixi/core/lib/core.js' implicitly has an 'any' type.
  Try `npm install @types/pixi__core` if it exists or add a new declaration (.d.ts) file containing `declare module '@pixi/core';`

ERROR in [at-loader] ./node_modules/pixi-tilemap/index.d.ts:13:29 
    TS7016: Could not find a declaration file for module '@pixi/constants'. 'repositories/pixi-tilemap-test/node_modules/@pixi/constants/lib/constants.js' implicitly has an 'any' type.
  Try `npm install @types/pixi__constants` if it exists or add a new declaration (.d.ts) file containing `declare module '@pixi/constants';`

ERROR in [at-loader] ./node_modules/pixi-tilemap/index.d.ts:14:24 
    TS7016: Could not find a declaration file for module '@pixi/core'. 'repositories/pixi-tilemap-test/node_modules/@pixi/core/lib/core.js' implicitly has an 'any' type.
  Try `npm install @types/pixi__core` if it exists or add a new declaration (.d.ts) file containing `declare module '@pixi/core';`

ERROR in [at-loader] ./node_modules/pixi-tilemap/index.d.ts:15:24 
    TS7016: Could not find a declaration file for module '@pixi/sprite'. 'repositories/pixi-tilemap-test/node_modules/@pixi/sprite/lib/sprite.js' implicitly has an 'any' type.
  Try `npm install @types/pixi__sprite` if it exists or add a new declaration (.d.ts) file containing `declare module '@pixi/sprite';`

ERROR in [at-loader] ./node_modules/pixi-tilemap/index.d.ts:16:25 
    TS7016: Could not find a declaration file for module '@pixi/core'. 'repositories/pixi-tilemap-test/node_modules/@pixi/core/lib/core.js' implicitly has an 'any' type.
  Try `npm install @types/pixi__core` if it exists or add a new declaration (.d.ts) file containing `declare module '@pixi/core';`
ℹ ｢wdm｣: Failed to compile.

However these packages dont exist..

https://github.com/spassvogel/pixi-tilemap-test