lib-jitsi-meet
lib-jitsi-meet copied to clipboard
fix(package.json) Replace 'module' field with 'export' in package.json
This PR updates package.json and replaces currently used 'module' field with a recommended by NodeJS 'export' field. According to documentation the 'module' field is ignored by NodeJS and only used by bundlers. After changing it however everything seems to be building fine.
The reasoning behind this change:
- Currently when installing the package via npm using a link to a released archive, for example: https://github.com/jitsi/lib-jitsi-meet/releases/download/v1486.0.0+73bc6ba2/lib-jitsi-meet.tgz importing the lib throws a build error as it can't locate the module, given 'module' field being ignored by NodeJS and there is no index.js in the root directory.
With this change it's possible to import the package without any hiccups:
import JitsiMeetJS from 'lib-jitsi-meet';
I noticed that in jitsi-meet app you're importing the package like that already, but I'm not entirely sure why it works for you, perhaps you got some extra configuration that resolves modules by looking at that 'module' field 🤔
Hi, thanks for your contribution! If you haven't already done so, could you please make sure you sign our CLA (https://jitsi.org/icla for individuals and https://jitsi.org/ccla for corporations)? We would unfortunately be unable to merge your patch unless we have that piece :(.
Have you verified the mobile app also builds ok?
I just run the app on iOS and build via metro went just fine.
What's interesting though, it shouldn't build due to Metro not supporting "exports" fields yet (https://github.com/facebook/metro/issues/670), however conducting more experiments I noticed that metro actually takes the package from the "browser" field, not "modules". Even in log messages it seems like they treat "browser" as "main" field (if we specify wrong package in "browser" field - the error from metro tells that field "main" points to a non-existent package 🤷)
Btw. when you work on the lib, how do you link it to the mobile app locally? Metro doesn't seem to follow symlinks so can't do typical npm link ..., or am I missing something?
Another option would be to add the react-native field to make sure RN prefers the ESM bundle, which is what it uses today.
Btw. when you work on the lib, how do you link it to the mobile app locally? Metro doesn't seem to follow symlinks so can't do typical
npm link ..., or am I missing something?
You can use install-local for example.
Replacing 'browser' with 'react-native' and pointing to ESM module works fine, though it throws one warning about circular dependency. Do you want me to fix it as part of this PR too?
I didn't get that warning using current version of the lib, so I'm thinking that RN currently uses the UMD version of the lib, not the ESM.

Hum, I was convinced we were using the ESM build, I may be wrong.
Yeah we should get the cycle fixed if that's the only one.
I fixed the circular dependency in latest commit @saghul
Should I change anything, or we can consider this fix complete? :)