Skip to content
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

Blocking bug with @pixi/node, unable to use with non-CommonJS modules #8502

Closed
3 tasks done
bigtimebuddy opened this issue Jul 16, 2022 · 6 comments · Fixed by #8505 or #8506
Closed
3 tasks done

Blocking bug with @pixi/node, unable to use with non-CommonJS modules #8502

bigtimebuddy opened this issue Jul 16, 2022 · 6 comments · Fixed by #8505 or #8506
Labels
🕷 Bug Verified that it’s actually a legit bug that exists in the current release. 🔥 High Priority These are PRs or issues that are extremely important to address either because they are from sponsor
Milestone

Comments

@bigtimebuddy
Copy link
Member

bigtimebuddy commented Jul 16, 2022

I've been experimenting with the @pixi/node package available in v6.5.0-rc.2. I have no problems when it's using CommonJS (e.g., require('@pixi/node')), but I am having issues using modules (e.g., import * as PIXI from '@pixi/node').

Here's a simple example:

https://github.com/bigtimebuddy/pixi-node-example/blob/module/index.mjs

Running this produces the following error:

import { Extract } from '@pixi/extract';
         ^^^^^^^
SyntaxError: Named export 'Extract' not found. The requested module '@pixi/extract' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from '@pixi/extract';
const { Extract } = pkg;

    at ModuleJob._instantiate (internal/modules/esm/module_job.js:124:21)
    at async ModuleJob.run (internal/modules/esm/module_job.js:179:5)
    at async Loader.import (internal/modules/esm/loader.js:178:24)
    at async Object.loadESM (internal/process/esm_loader.js:68:5)
    at async handleMainPromise (internal/modules/run_main.js:59:12)

Having all packages be both CommonJS and ESM seems to confuse Node, even with the proper type. I have not found a way to force dependencies to use modules. Kinda stuck, any ideas?

A workaround would be to bundle everything (except the 3rd party dependencies like canvas, gl, etc) so that it's a single file. Providing cjs and mjs files seems like it would work.


EDIT: think I figured out what needs to happen here:

TODO

@bigtimebuddy bigtimebuddy added 🕷 Bug Verified that it’s actually a legit bug that exists in the current release. 🔥 High Priority These are PRs or issues that are extremely important to address either because they are from sponsor labels Jul 16, 2022
@bigtimebuddy bigtimebuddy added this to the v6.5.0 milestone Jul 16, 2022
@bigtimebuddy
Copy link
Member Author

Another workaround, is to change all the esm packages to have ".mjs" extension instead of ".js".

@bigtimebuddy
Copy link
Member Author

I was able to patch exports.import.default on every package and replace .mjs and it worked, but I'm now running into another issue related to ismobilejs:

var isMobile = isMobileCall(globalThis.navigator);
               ^

TypeError: isMobileCall is not a function
    at ~/pixi-node-example/node_modules/@pixi/settings/dist/esm/settings.mjs:545:16
    at ModuleJob.run (node:internal/modules/esm/module_job:193:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:541:24)
    at async loadESM (node:internal/process/esm_loader:83:5)
    at async handleMainPromise (node:internal/modules/run_main:65:12)

Here's the offending output:

var isMobile = isMobileCall(globalThis.navigator);

Replaced with this worked.

var isMobile = isMobileCall.default(globalThis.navigator);

We need to find a better way to import that dependency, maybe bundling?

@miltoncandelero
Copy link
Contributor

Not sure about the dependency stuff however isMobile is not node friendly, it doesn't know that is running in a backend environment. Maybe is-mobile might work best and it knows to read the headers of http requests? 🤔

@bigtimebuddy
Copy link
Member Author

I'm thinking that ismobilejs is not applicable for this usage. We use it internally most to check for apple or android devices. We might be better served moving it to the Browser and Node adapters instead.

I finally got hello world working with mjs, but it required these ugly hacks to fix the above things:
https://github.com/bigtimebuddy/pixi-node-example/blob/module/fix.mjs

These things seem fixable.

@Julien-Marcou
Copy link
Member

Julien-Marcou commented Jul 17, 2022

NodeJS requires a .mjs extension or "type": "module" inside the package.json in order to import a file as an ES Module.

I don't know if it's possible to have several package.json, one per distribution.

EDIT:

Looks like it's possible: https://www.sensedeep.com/blog/posts/2021/how-to-create-single-source-npm-module.html

For each packages, adding a dist/esm/package.json, that contains:

{
  "type": "module"
}

And a dist/cjs/package.json that contains:

{
  "type": "commonjs"
}

Seems to be a solution.

@UltraCakeBakery
Copy link

UltraCakeBakery commented Jul 18, 2022

I came across https://antfu.me/posts/publish-esm-and-cjs a while ago when I was running into this issue with a side project I'm working on. The configurations used in the package.json might be helpful here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🕷 Bug Verified that it’s actually a legit bug that exists in the current release. 🔥 High Priority These are PRs or issues that are extremely important to address either because they are from sponsor
Projects
None yet
4 participants