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

Npmrc #3135

Merged
merged 8 commits into from
Aug 20, 2023
Merged

Npmrc #3135

merged 8 commits into from
Aug 20, 2023

Conversation

bugsounet
Copy link
Contributor

Fix engines check on npm install

it will alow to check engine requirement of package.json

actually:

	"engines": {
		"node": ">=16"
	}

if requirements are not suitable, it's break installer

bugsounet@Kubuntu:~/MagicMirror-dev$ npm install
npm ERR! code EBADENGINE
npm ERR! engine Unsupported engine
npm ERR! engine Not compatible with your version of node/npm: magicmirror@2.24.0-develop
npm ERR! notsup Not compatible with your version of node/npm: magicmirror@2.24.0-develop
npm ERR! notsup Required: {"node":">=16"}
npm ERR! notsup Actual:   {"npm":"x.y.z","node":"v14.xx.yy"}

actually, it's just a warn:

bugsounet@KUbuntu:~/MagicMirror-dev$ npm install
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: 'magicmirror@2.24.0-develop',
npm WARN EBADENGINE   required: { node: '>=16' },
npm WARN EBADENGINE   current: { node: 'v14.xx.yy', npm: 'x.y.z' }
npm WARN EBADENGINE }
...

and don't break installation, this can causes errors in the main core of MM²

@bugsounet
Copy link
Contributor Author

bugsounet commented Jun 25, 2023

by the way, electron v25.2.0 is out

main core node version is v18
don't you turn engine to node v18 ?

@codecov-commenter
Copy link

codecov-commenter commented Jun 25, 2023

Codecov Report

Merging #3135 (d071e00) into develop (7c64d8f) will increase coverage by 0.08%.
The diff coverage is n/a.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@             Coverage Diff             @@
##           develop    #3135      +/-   ##
===========================================
+ Coverage    26.15%   26.24%   +0.08%     
===========================================
  Files           53       53              
  Lines        11500    11500              
===========================================
+ Hits          3008     3018      +10     
+ Misses        8492     8482      -10     

see 2 files with indirect coverage changes

@khassel
Copy link
Collaborator

khassel commented Jun 26, 2023

by the way, electron v25.2.0 is out

meanwhile merged

main core node version is v18 don't you turn engine to node v18 ?

we will update the docs with next release, see MagicMirrorOrg/MagicMirror-Documentation#163

@khassel khassel requested a review from rejas June 26, 2023 19:51
@bugsounet
Copy link
Contributor Author

bugsounet commented Jun 26, 2023

so, in package.json:

	"engines": {
		"node": ">=18"
	}

Humm If i add it: i will break suite test for node v16

In node v16 suite test :

Run npm run install-mm:dev

> magicmirror@2.2[4](https://github.com/MichMich/MagicMirror/actions/runs/5382606508/jobs/9768261543?pr=3135#step:4:5).0-develop install-mm:dev
> npm install --no-audit --no-fund --no-update-notifier

npm ERR! code EBADENGINE
npm ERR! engine Unsupported engine
npm ERR! engine Not compatible with your version of node/npm: magicmirror@2.24.0-develop
npm ERR! notsup Not compatible with your version of node/npm: magicmirror@2.24.0-develop
npm ERR! notsup Required: {"node":">=18"}
npm ERR! notsup Actual:   {"npm":"8.19.4","node":"v1[6](https://github.com/MichMich/MagicMirror/actions/runs/5382606508/jobs/9768261543?pr=3135#step:4:7).20.0"}

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/runner/.npm/_logs/2023-06-26T20_13_20_[7](https://github.com/MichMich/MagicMirror/actions/runs/5382606508/jobs/9768261543?pr=3135#step:4:8)9[8](https://github.com/MichMich/MagicMirror/actions/runs/5382606508/jobs/9768261543?pr=3135#step:4:9)Z-debug-0.log
Error: Process completed with exit code 1.

@khassel
Copy link
Collaborator

khassel commented Jun 26, 2023

I think we should not force all current node16 installations to use node18 with the new release.

We will update the docs to use node 18 but the engine section above should stay with >=16 (if we use strict) until node 16 is out of maintenance in Sep. 2023 (see https://endoflife.date/nodejs). I'm open to better ideas.

@bugsounet
Copy link
Contributor Author

bugsounet commented Jun 26, 2023

don't forget next release of MM² (v2.25.x) will be in october 2023.
node v16 is out of maintenance in Sep. 2023

Maybe changing now is the best way no ?

why ?
Electron 25.2.x use node v18.15.0 in main core

image

@khassel
Copy link
Collaborator

khassel commented Jun 26, 2023

Maybe changing now is the best way no ?

from my side this is o.k., I'm already on v18 in my docker images (and on v20 in develop images).

If @rejas and @sdetweil agree then the node 16 test must be removed in the github workflow and there are 2 *.md files in the .github folder where 16 should be replace by 18.

@rejas
Copy link
Collaborator

rejas commented Jun 26, 2023

I tend to lean on the "keep node16 until next release in october". Mostly because with only 5 days away from the next release I want to keep the changes at a bare minimum.
One month of unmaintained node16 shoudlnt be too bad, but maybe if @sdetweil wants to turn the scales ;-)

@sdetweil
Copy link
Collaborator

I will update installer and upgrader to use 18

@sdetweil
Copy link
Collaborator

yes agree to move to 18,

it's less than a week of testing tho

@bugsounet
Copy link
Contributor Author

@sdetweil : If you are able to do it, why not
otherwise follow the advice of Veeck

@rejas: but be careful according to the package-lock.json (sub-dependencies)
the minimum node version to use is v16.13.0
cat package-lock.json | grep 'node":' | grep 16

Remember that we ask in package.json node >=16.0.0 for MM²

What we do ?

@rejas
Copy link
Collaborator

rejas commented Jun 30, 2023

Can you test this @sdetweil ? I dont have the time and I am not sure its feasible for tomorrows release....

@bugsounet
Copy link
Contributor Author

@rejas: I think it's better to take time.
Don't worry, we see this for next release ;)

@bugsounet
Copy link
Contributor Author

@rejas: we can add audit=false for not check vulnerabilities

why ?

Because some user like npm audit command and can make some package trouble

@rejas
Copy link
Collaborator

rejas commented Aug 20, 2023

@rejas: we can add audit=false for not check vulnerabilities

why ?

Because some user like npm audit command and can make some package trouble

Make sense. We developers do the dependency updates anyway on a regular basis.

Will you add it to the branch?

don't check vulnerabilities
@bugsounet
Copy link
Contributor Author

added

Generaly, bots make jobs
but we can check it manualy with npm audit

@rejas rejas merged commit 7ba96ae into MagicMirrorOrg:develop Aug 20, 2023
7 checks passed
@bugsounet bugsounet deleted the npmrc branch September 8, 2023 08:01
@MichMich MichMich mentioned this pull request Oct 1, 2023
MichMich added a commit that referenced this pull request Oct 1, 2023
## [2.25.0] - 2023-10-01

Thanks to: @bugsounet, @dgoth, @dependabot, @kenzal, @Knapoc,
@KristjanESPERANTO, @martingron, @NolanKingdon, @Paranoid93,
@TeddyStarinvest and @Ybbet.

Special thanks to @khassel, @rejas and @sdetweil for taking over most
(if not all) of the work on this release as project collaborators. This
version would not be there without their effort. Thank you guys! You are
awesome!

> ⚠️ This release needs nodejs version >= `v18`, older releases have
reached end of life and will not work!

### Added

- Added UV Index support to OpenWeatherMap
- Added 'hideDuplicates' flag to the calendar module
- Added `allowOverrideNotification` to weather module to enable sending
current weather objects with the `CURRENT_WEATHER_OVERRIDE` notification
to supplement/replace the current weather displayed
- Added optional AnimateCSS animate for `hide()`, `show()`,
`updateDom()`
- Added AnimateIn and animateOut in module config definition
- Apply AnimateIn rules on the first start
- Added automatic client page reload when server was restarted by
setting `reloadAfterServerRestart: true` in `config.js`, per default
`false` (#3105)
- Added eventClass option for customEvents on the default calendar
- Added AnimateCSS integration in tests suite (#3206)
- Added npm dependabot [Reserved to developer] (#3210)
- Added improved logging for calendar (#3110)

### Removed

- **Breaking Change**: Removed `digest` authentication method from
calendar module (which was already broken since release `2.15.0`)

### Updated

- Update roboto fonts to version v5
- Update issue template
- Update dev/dependencies incl. electron to v26
- Replace pretty-quick by lint-staged
(<prettier/pretty-quick#164>)
- Update engine node >=18. v16 reached it's end of life. (#3170)
- Update typescript definition for modules
- Cleaned up nunjuck templates
- Replace `node-fetch` with internal fetch (#2649) and remove
`digest-fetch`
- Update the French translation according to the English file.
- Update dependabot incl. vendor/fonts (monthly check)
- Renew `package-lock.json` for release

### Fixed

- Fix engine check on npm install (#3135)
- Fix undefined formatTime method in clock module (#3143)
- Fix clientonly startup fails after async added (#3151)
- Fix electron width/heigth when using xrandr under bullseye
- Fix time issue with certain recurring events in calendar module
- Fix ipWhiteList test (#3179)
- Fix newsfeed: Convert HTML entities, codes and tag in description
(#3191)
- Respect width/height (no fullscreen) if set in electronOptions
(together with `fullscreen: false`) in `config.js` (#3174)
- Fix: AnimateCSS merge hide() and show() animated css class when we do
multiple call
- Fix `Uncaught SyntaxError: Identifier 'getCorsUrl' has already been
declared (at utils.js:1:1)` when using `clock` and `weather` module
(#3204)
- Fix overriding `config.js` when running tests (#3201)
- Fix issue in weathergov provider with probability of precipitation not
showing up on hourly or daily forecast

---------

Signed-off-by: naveen <172697+naveensrinivasan@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Karsten Hassel <hassel@gmx.de>
Co-authored-by: Malte Hallström <46646495+SkySails@users.noreply.github.com>
Co-authored-by: Veeck <github@veeck.de>
Co-authored-by: veeck <michael@veeck.de>
Co-authored-by: dWoolridge <dwoolridge@charter.net>
Co-authored-by: Johan <jojjepersson@yahoo.se>
Co-authored-by: Dario Mratovich <dario_mratovich@hotmail.com>
Co-authored-by: Dario Mratovich <dario.mratovich@outlook.com>
Co-authored-by: Magnus <34011212+MagMar94@users.noreply.github.com>
Co-authored-by: Naveen <172697+naveensrinivasan@users.noreply.github.com>
Co-authored-by: buxxi <buxxi@omfilm.net>
Co-authored-by: Thomas Hirschberger <47733292+Tom-Hirschberger@users.noreply.github.com>
Co-authored-by: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com>
Co-authored-by: Andrés Vanegas Jiménez <142350+angeldeejay@users.noreply.github.com>
Co-authored-by: Dave Child <dave@addedbytes.com>
Co-authored-by: grenagit <46225780+grenagit@users.noreply.github.com>
Co-authored-by: Grena <grena@grenabox.fr>
Co-authored-by: Magnus Marthinsen <magmar@online.no>
Co-authored-by: Patrick <psieg@users.noreply.github.com>
Co-authored-by: Piotr Rajnisz <56397164+rajniszp@users.noreply.github.com>
Co-authored-by: Suthep Yonphimai <tomzt@users.noreply.github.com>
Co-authored-by: CarJem Generations (Carter Wallace) <cwallacecs@gmail.com>
Co-authored-by: Nicholas Fogal <nfogal.misc@gmail.com>
Co-authored-by: JakeBinney <126349119+JakeBinney@users.noreply.github.com>
Co-authored-by: OWL4C <124401812+OWL4C@users.noreply.github.com>
Co-authored-by: Oscar Björkman <17575446+oscarb@users.noreply.github.com>
Co-authored-by: Ismar Slomic <ismar@slomic.no>
Co-authored-by: Jørgen Veum-Wahlberg <jorgen.wahlberg@amedia.no>
Co-authored-by: Eddie Hung <6740044+eddiehung@users.noreply.github.com>
Co-authored-by: Bugsounet - Cédric <github@bugsounet.fr>
Co-authored-by: bugsounet <bugsounet@bugsounet.fr>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Knapoc <Knapoc@users.noreply.github.com>
Co-authored-by: sam detweiler <sdetweil@gmail.com>
Co-authored-by: veeck <michael.veeck@nebenan.de>
Co-authored-by: Paranoid93 <6515818+Paranoid93@users.noreply.github.com>
Co-authored-by: NolanKingdon <27908974+NolanKingdon@users.noreply.github.com>
Co-authored-by: J. Kenzal Hunter <kenzal.hunter@gmail.com>
Co-authored-by: Teddy <teddy.payet@gmail.com>
Co-authored-by: TeddyStarinvest <teddy.payet@starinvest.com>
Co-authored-by: martingron <61826403+martingron@users.noreply.github.com>
Co-authored-by: dgoth <132394363+dgoth@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants