NPM Resource Changed ? Pending

User tests: Successful: Unsuccessful:

avatar dgrammatiko
dgrammatiko
25 Nov 2022

Pull Request for Issue # .

Summary of Changes

The Browserlist db needs to be updated periodically.

  • A new script was added in the package.json: "browserlist:update": "npx browserslist@latest --update-db"
  • An action to automate this. Running every Sunday

Check the npm action on the drone:
Screenshot 2022-11-25 at 22 52 23

Testing Instructions

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@HLeithner @wilsonge @Hackwar please check the action part, you might need to tweak it

avatar dgrammatiko dgrammatiko - open - 25 Nov 2022
avatar dgrammatiko dgrammatiko - change - 25 Nov 2022
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 25 Nov 2022
Category Repository NPM Change
avatar dgrammatiko dgrammatiko - change - 25 Nov 2022
The description was changed
avatar dgrammatiko dgrammatiko - edited - 25 Nov 2022
avatar HLeithner
HLeithner - comment - 25 Nov 2022

Makes no sense, if it's not already done in the build script then it should be part of the build script like the webauthn update. Also we update the browser list in composer, is this another needed list?

avatar wilsonge
wilsonge - comment - 26 Nov 2022

This is the library that nearly everything uses to power the js/css build tooling to compile for the right versions https://github.com/joomla/joomla-cms/blob/4.2-dev/package.json#L30-L33 here. You'd use the same thing for webpack/rollup/parcel etc. So yes it's definitely required. What browser thing do we have in composer? Not aware of one?

The thing we use composer for regularly is the CA Cert list updates

avatar dgrammatiko
dgrammatiko - comment - 26 Nov 2022

Testing should be easy: npm ci before applying the patch, make a backup of the media folder then apply the patch and run npm ci and then compare the 2 folders.

About https://github.com/joomla/joomla-cms/blob/4.2-dev/package.json#L30-L33 I guess you could copy the browser list from Bootstrap which is less radical https://github.com/twbs/bootstrap/blob/main/.browserslistrc. FWIW js is not really impacted here as the tools compile to es5 for the no modules/legacy and es2018 for the modules

avatar dgrammatiko
dgrammatiko - comment - 26 Nov 2022

Makes no sense, if it's not already done in the build script

Objection! Sorry @HLeithner but that would create a huge disconnect between what people actually tested with what the project will release. Let me explain:

  • the script updates only the zip file od the browser list db in the package.lock.json

Screenshot 2022-11-26 at 09 36 25
so anyone that either downloads the package from a PR or they run npm ci they will use the same db entries as the one that the project will use for the production

  • If you just run the update prior to release you risk having slight differences (mainly on CSS) and this due to the fact that the browser list is way too loose (2 versions).

FWIW George spotted this when I was updating the custom elements repo, thus the PR here

avatar HLeithner
HLeithner - comment - 26 Nov 2022

OK then it can only be done like the composer update in a minor release.

avatar dgrammatiko
dgrammatiko - comment - 26 Nov 2022

OK then it can only be done like the composer update in a minor release.

I'm not the one to tell you what the frequency that the action should run. Although since my idea was to automate this it implies that it should run more frequently. As I said copy the https://github.com/twbs/bootstrap/blob/main/.browserslistrc from the Bootstrap repo (which is anchored to specific versions) and let the action run as frequent as possible.

Also a bit unrelated bu since I was checking the other actions I think you should manage the node version and the base branch using the repo's secrets. That way you don't have to touch the actual scripts when either of those is changed, eg:

  • this node-version: 16 becomes node-version: ${{ secrets.NODE_VERSION }}
  • this -base 4.2-dev becomes -base ${{ secrets.BASE_BRANCH }}

and you need to add NODE_VERSION , and BASE_BRANCH in the secrets (with values 16 and 4.2-dev). Then for 4.3-dev you just update the values in the UI, not touching the actual scripts.

avatar HLeithner
HLeithner - comment - 27 Nov 2022

So we can you remove the github action please and only keep the browserlist:update command?

I added the step to the beta1 of a minor/major release in the release checklist documentation https://github.com/joomla/internal-documentation/pull/24

avatar dgrammatiko dgrammatiko - change - 27 Nov 2022
Labels Added: NPM Resource Changed ?
avatar joomla-cms-bot joomla-cms-bot - change - 27 Nov 2022
Category Repository NPM Change NPM Change
avatar dgrammatiko
dgrammatiko - comment - 27 Nov 2022

@HLeithner done, only the script is left here. Btw I don't have access to the internal docs repo but I trust you and George are getting it right

avatar HLeithner HLeithner - change - 27 Nov 2022
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2022-11-27 09:03:03
Closed_By HLeithner
avatar HLeithner HLeithner - close - 27 Nov 2022
avatar HLeithner HLeithner - merge - 27 Nov 2022
avatar HLeithner
HLeithner - comment - 27 Nov 2022

Thanks, and I invited you

Add a Comment

Login with GitHub to post a comment