Any plan to replace "request" and "moment"?

sdetweil

@0m4r we have already moved away from request… it was removed in 2.16

I hadn’t seen moment status
I think we moved to moment.tz in calendar

karsten13

I think we should open an issue in the github repo to discuss the moment stuff there, if it should be replaced and with what. Found this article.

In the current mm release 2.16.0 request is still a dev dependency but it is now totally removed on the develop branch (and so in the next release).

BKeyport

My concern as a module handler that has two modules that currently use Moment, how do I switch, either now or later?

sdetweil

@bkeyport now, and later, use the new library in your module and add it to your package.json.

then if the system provides it npm install will say ‘done’
and if the system doesn’t, then it will load it for your module…

mumblebaj

@sdetweil Just a question from myself, is there a preference from the platform side on what to use to replace request? I have just updated my module to use node-fetch. Are there preferences from the MMM platform side?

sdetweil

@mumblebaj no preference…

I use axios

see this list
https://github.com/request/request/issues/3143

mumblebaj

@sdetweil Thanks Sam. Yeah, I saw there are a few replacements for request. I had a look at axios previously as well.

0m4r

@mumblebaj there are many options. I am planning to replace all with node-fetch (https://github.com/node-fetch/node-fetch), and concerning moment, I think I’d go for date-fns: https://date-fns.org/

0m4r

@sdetweil ah, I had an old version that still included “request”. ID’ migrate my modules over to node-fetch!

About moment, I’d recommend taking a look at date-fns, many are moving over to it to replace moment - even tho the refactoring effort may not be trivial, the advantages seem to be more (including the smaller size of the final bundle)!

0m4r

@karsten13 said in Any plan to replace "request" and "moment"?:

In the current mm release 2.16.0 request is still a dev dependency but it is now totally removed on the develop branch (and so in the next release).

this would be a breaking change for many modules, what’s the plan to communicate it to the module developers (and potentially, to the users?)

karsten13

@0m4r we had similar discussions already here

swvalenti

How can I tell if a module will be compatible prior to the fall upgrade of 2.17.0? I don’t want to upgrade and half of my modules break.

0m4r

@swvalenti if your module uses “request” to issue HTTP calls then it is.
so, if you import it in your node_helper.js (or an alike node pice of code).

i.e.
const request = require('request');
or
import request from 'request'

on way to preserve it is to add “request” as a dependency to your module, but your end-users will need to install them on their own.
To add it to your module, from its folder you can do:
npm install request ; this will update your package.json file (and package-lock.json) - commit the change to your git repo (this is most likely a breaking change and would require a major version increase - if you version your code!)

Once this is done, your existing (and new) end-user will need to update their copy of your module and run npm install on their own to download request and enable them to use it.

swvalenti

@0m4r said in Any plan to replace "request" and "moment"?:

const request = require(‘request’);

thank you for the explanation I understand better now

karsten13

@swvalenti one addition: Only modules are affected which rely on that request is intalled in core mm (which is a dirty hack of the module developer), modules which install request on their own are not!

sdetweil

@karsten13 said in Any plan to replace "request" and "moment"?:

which is a dirty hack of the module developer

no… don’t say it that way…

lack of experience on the part of the developer…

it just worked, so they didn’t know to ask if there was a more correct way…

I had that problem early on in my nodejs development too…

karsten13

@sdetweil doing this in some of my own modules so I’m allowed to say this ;)

sdetweil

@karsten13 doing it ON PURPOSE is a hack!!

0m4r

@sdetweil @sdetweil I am not convinced about both of your points of view. I mean technically yes.

But… looking into the modules themselves, they are solely made to work with the MagicMirror code base (99% of the cases)
What is the point to bloat the size of the installation with the same node module installed over and over again? when on, “global” can be used and relied upon? (well, yeah, I agree, “this situation” is one reason for it!). But why trade-off convenience over complexity?

I see it as a part of the Magic Mirror ecosystem, provide some common “code” to be used by everyone to both ease the development and the set-up of it.

As mentioned in the other thread, old legacy modules will probably stop working - forking them or re-doing them will fix them. This not only pushes forward the advancement of the whole MagicMirror tech stack, but it may strengthen the community and really push it to hold together and upgrade, modernize the code of the modules.

bottom line, nobody says it is going to be easy, but it is a big opportunity.

sdetweil

@0m4r said in Any plan to replace "request" and "moment"?:

What is the point to bloat the size of the installation with the same node module installed over and over again

it is NOT installed over and over

npm install start with the module folder and says

is it here?   
   no   
     look in the parent folders, 1 by 1, looking for a node_modules folder
       is it there?
          yes, stop
   yes, 
     stop.

so it will only be installed once…

in the near term with all the old modules ‘assuming’ is was installed,
it’ll have to be installed in the MM/node_modules (but only once)