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

Some of dependencies are removed from headless package.json, but still required #1352

Closed
cmrd-senya opened this issue Nov 23, 2018 · 6 comments
Labels
headless @converse/headless question
Milestone

Comments

@cmrd-senya
Copy link
Member

At 3f7ffd0#diff-8d8848d744f09155a6ed06111726618c some of dependencies have been removed, but they are still required.

For instance jed is not in the package.json anymore, but it is required in https://github.com/conversejs/converse.js/blob/master/src/headless/i18n.js which is required in https://github.com/conversejs/converse.js/blob/master/src/headless/converse-core.js. Therefore the only way to use @converse/headless is to manually add dependencies like jed to your local dependencies.

We should either remove the depending code from @converse/headless or add the dependencies to package.json back again.

@jcbrand
Copy link
Member

jcbrand commented Nov 30, 2018

@cmrd-senya What do you think? jed is used for translations. I'm not sure whether something like that should go into the headless build or not.

@cmrd-senya
Copy link
Member Author

Module i18n depends on jed and currently is a part of the headless build. The only thing why i18n is referenced in the converse-core module is localization initialization. I think that we can extract this code out from the converse-core and move it somewhere else.

@jcbrand jcbrand added the headless @converse/headless label Mar 8, 2019
@kevinfaveri
Copy link

kevinfaveri commented Mar 13, 2019

Just to leave recorded here: while playing with @converse/headless (https://www.npmjs.com/package/@converse/headless) I tried a simple example like this:

import "@converse/headless/converse-pubsub";      // XEP-0199 XMPP Ping
import "@converse/headless/converse-chatboxes";   // Backbone Collection and Models for chat boxes
import "@converse/headless/converse-disco";       // Service discovery plugin
import "@converse/headless/converse-mam";         // XEP-0313 Message Archive Management
import "@converse/headless/converse-ping";        // XEP-0199 XMPP Ping
import "@converse/headless/converse-roster";      // Contacts Roster
import "@converse/headless/converse-vcard";       // XEP-0054 VCard-temp

import converse from "@converse/headless/converse-core";

When running this code these were the missing packages:

  • backbone
  • es6-promise
  • jed
  • moment
  • pluggable.js
  • sizzle
  • backbone.browserStorage
  • backbone.overview
  • backbone.vdomview
  • strophejs-plugin-ping
  • jquery

After installing all these the warnings in build time had gone.


However, if I add this import:

import "@converse/headless/converse-muc";         // XEP-0045 Multi-user chat

I receive these errors:

 ERROR  Failed to compile with 7 errors    
These dependencies were not found:
* snabbdom in ./node_modules/backbone.vdomview/backbone.vdomview.js         
* snabbdom-attributes in ./node_modules/backbone.vdomview/backbone.vdomview.js    
* snabbdom-class in ./node_modules/backbone.vdomview/backbone.vdomview.js   
* snabbdom-dataset in ./node_modules/backbone.vdomview/backbone.vdomview.js  
* snabbdom-props in ./node_modules/backbone.vdomview/backbone.vdomview.js       
* snabbdom-style in ./node_modules/backbone.vdomview/backbone.vdomview.js         
* tovnode in ./node_modules/backbone.vdomview/backbone.vdomview.js      

@jcbrand
Copy link
Member

jcbrand commented Mar 14, 2019

@kevinfaguiar: backbone, es6-promise, moment, pluggable.js are all set as dependencies in the package.json of @converse/headless.

See here: https://github.com/conversejs/converse.js/blob/master/src/headless/package.json#L25

jQuery is not a dependency of @converse/headless and is not used, so I don't know why you think it's a missing dependency.

Concerning the snabdom errors, that's because you need webpack aliases.
See here: https://github.com/conversejs/converse.js/blob/master/webpack.config.js#L91

We should probably document that somewhere, or make a fix so that those aliases are no longer necessary.

@kevinfaveri
Copy link

kevinfaveri commented Mar 14, 2019

@kevinfaguiar: backbone, es6-promise, moment, pluggable.js are all set as dependencies in the package.json of @converse/headless.

See here: https://github.com/conversejs/converse.js/blob/master/src/headless/package.json#L25

The problem here, I think, is that the dependencies are listed under a "devDependencies" tag in package.json. So, when I install @converse/headless as a dependency to my project these devDependencies are not fetched... I think this is the default behaviour of using NPM...

A simple fix would be changing "devDependencies" to "dependencies" here, since @converse/headless is not a distrib file: https://github.com/conversejs/converse.js/blob/master/src/headless/package.json#L24

A complex one would be building the @converse/headless into a single distrib file, which would demand extra work and code rewrite to make the modules (like "@converse/headless/converse-chatboxes") plugabble and build it into a single js distrib file with webpack...


jQuery is not a dependency of @converse/headless and is not used, so I don't know why you think it's a missing dependency.

It seems to me it is this "jquery", probably solved with the alias (I wasn't aware of this alias until now) :

"jquery": path.resolve(__dirname, "src/jquery-stub"),


Concerning the snabdom errors, that's because you need webpack aliases.
See here: https://github.com/conversejs/converse.js/blob/master/webpack.config.js#L91

We should probably document that somewhere, or make a fix so that those aliases are no longer necessary.

Yeah, I wasn't aware these alias were necessary. Maybe using relative paths to fix the problem?


EDIT: Something I've missed? My line of thinking was that https://www.npmjs.com/package/@converse/headless should be the headless build file, maybe the distrib file complete and with no dependencies as done here:

if (type === 'headless') {

But apparently it is a project to setup the building of the headless? Because if is, you see, a lot of files from these aliases, like "jquery" (
"jquery": path.resolve(__dirname, "src/jquery-stub"),
), webpack and even Makefile are missing when i download the dependency with:
npm install @converse/headless
There is no "dist/converse-headless.js" too...


EDIT2: I was, following a commonly pattern in NPM, thinking the "@conversejs/headless" package, would import the full headless build "dist/converse-headless.js", so I could just import and start using.

But after downloading the conversejs project and building with "make dist/converse-headless.js" command I got the full build (aprox. 2MB)...

So I'm thinking... shouldn't the https://www.npmjs.com/package/@converse/headless export the full conversejs headless build (the one created with "make dist/converse-headless.js") to facilitate the integration into projects rather than the "src/headless/" directory? Or maybe add a directory "dist" in the "src/headless/" folder containing the headless build?

And then leave a documentation about how build and modify the headless.js so you can shrink the build only if needed?

@jcbrand
Copy link
Member

jcbrand commented Mar 15, 2019

Hi @kevinfaguiar,

thanks for your detailed response and for your patience in trying to use the headless build.

You're right, we need to have a distribution build of converse-headless.js to be used by 3rd party projects.

I've now made the relevant changes (see here) so that there is a file dist/converse-headless.js inside @converse/headless and the main key of the package.json points to it.

I've also created a quick and dirty headless-example which uses @converse/headless and which you can look at.

Because these changes are not released yet, you'll need to clone this repo and then npm link in @converse/headless so that you can have the latest code in your project. The headless-example README explains how you can do this.

I'm also closing this issue because I've now added Jed as a dependency, which is what this was about. Feel free to create new issues if you run into any problems.

Good luck and let me know how it goes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
headless @converse/headless question
Projects
None yet
Development

No branches or pull requests

3 participants