Any reason why details-element-polyfill.js is in the code base

[kelson42]

Instead of being a dependence?

[ISNIT0]

This is copied from res/ to the ZIM file. This is the easier and simpler way to do things.
The alternative is installing it into node_modules, then hard-code a url like node_modules/details-element-polyfill/dist/index.js which would be more fragile and possibly give weird errors if node modules haven't been installed.

It also creates a new vector for someone to maliciously update a dependence

[kelson42]

This is copied from res/ to the ZIM file. This is the easier and simpler way to do things.
The alternative is installing it into node_modules, then hard-code a url like node_modules/details-element-polyfill/dist/index.js which would be more fragile and possibly give weird errors if node modules haven't been installed.

If node modules are not installed or not fully installed according to the json files, mwoffliner does not work properly anyway? That is why npm checks this properly, isn't it?

It also creates a new vector for someone to maliciously update a dependence

How would that be introduced without going through a PR review?

[kelson42]

There is a gold rule in code mgmt: never put into your repo something which is not your source code. This includes:

• automatically generated files
• code belonging to other projects
• ...

[kelson42]

I reopen the ticket as we still have a problem here:

$ zimcheck out/wikipedia_bm_all_2019-07.zim 
[INFO] Checking zim file out/wikipedia_bm_all_2019-07.zim
[INFO] Verifying Internal Checksum.. 
  [INFO] Internal checksum found correct
[INFO] Searching for metadata entries..
[INFO] Searching for Favicon..
[INFO] Searching for main page..
[INFO] Verifying Articles' content.. 
[INFO] Searching for redundant articles..
  Verifying Similar Articles for redundancies..
[ERROR] Invalid internal links found :
  -/j/node_modules/details-element-polyfill/dist/details-element-polyfill.js (../-/j/js_modules/../node_modules/details-element-polyfill/dist/details-element-polyfill.js) was not found in article A/(ߒߞߏ)
[INFO] Overall Test Status: Fail
[INFO] Total time taken by zimcheck: 2 seconds.

[kelson42]

@vaibhavmatta This is one which is quite easy and I would really like to get rid of it.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.