Webpacker: Use local NPM installer instead of duplicating the packages folder

[ahukkanen]

🎩 What? Why?

As discussed with @leio10 at #8136 (comment), we should migrate to using a local NPM package installer instead of duplicating the packages folder in every app.

This is a workaround for installing the linked package dependencies due to a bug in NPM v7 (npm/cli#2339). Until the issue is fixed at NPM, we need this workaround.

The local package installer is a separate NPM package implemented at:
https://github.com/decidim/decidim-npm-local

This tool does not have to follow the Decidim version numbering, it is designed to work on its own. It could be released already to NPM even before there is an actual Decidim release available.

The workaround works as follows:

• It fetches the decidim gem installation path using bundle show decidim
  • If it finds one, it copies the packages folder to the application's tmp folder which doesn't go to git
  • If it fails to find the Decidim installation path, it will default to installing the Decidim NPM dependencies from Decidim's git repository using degit
• It packs all the NPM dependencies to their own TGZ pack files
• It installs the packed dependencies from the TGZ packages which also installs the "linked" dependencies

Benefits compared to the current solution:

• The packages folder does not need to be duplicated to the application itself for local and git installations
• The packages folder does not need to be duplicated to the design app

If the related NPM bug is fixed at some point, we can consider removing this workaround and installing the Decidim NPM dependencies directly using file paths pointing to the decidim gem's installation folder (when available, note that there are also installations that do not install the decidim main gem but only e.g. decidim-core, decidim-admin, etc.).

📌 Related Issues

• Related to Fix webpacker issues #8136 (comment) Choose the right version of Decidim's package.json #8119 npm v7 does not install linked packages dependencies npm/cli#2339

Testing

• Create the development app
• Go to the development app
• Remove the tmp/npmbuild folder
• Remove the node_modules folder
• Run npm i inside the development app
• Start the webpack dev server by running ./bin/webpack-dev-server
• See that there are no problems running the server and the npm packs are always regenerated when running npm i causing no changes to e.g. deployment scripts

📋 Checklist

• ❓ CONSIDER adding a unit test if your PR resolves an issue.
• ✔️ DO check open PR's to avoid duplicates.
• ✔️ DO keep pull requests small so they can be easily reviewed.
• ✔️ DO build locally before pushing.
• ✔️ DO make sure tests pass.
• ✔️ DO make sure any new changes are documented in docs/.
• ✔️ DO add and modify seeds if necessary.
• ✔️ DO add CHANGELOG upgrade notes if required.
• ✔️ DO add to GraphQL API if there are new public fields.
• ✔️ DO add link to MetaDecidim if it's a new feature.
• ❌AVOID breaking the continuous integration build.
• ❌AVOID making significant changes to the overall architecture.

[leio10]

@ahukkanen Thanks for your amazing work! 🤩
I barely had time to think about this, but my only concern is the apps deployments, as I already told you. My idea is to try to deploy an app using this branch and if everything works well I'll merge it.

[ahukkanen]

@leio10 Yes, I answered to your concern at #8136 (comment).

This approach means no changes to the deployment scripts as far as I know, if you are already running npm install during deployment.

When you run npm install (after the local installer is added as an NPM dependency), the Decidim packs are generated during that command and then installed locally. Therefore, I think there should be no changes needed to deployment scripts. It is essentially doing what develop is doing right now but through NPM. And thanks to this, the packages folder can live under tmp which is not committed to the repository.

[leio10]

Hi @ahukkanen! I'm trying to deploy this to Heroku and I'm getting an error message. The staging app is working with the packages folder. I've created a Review app with your branch, but it's failing. There is a GH Action that changes the Gemfile to point to your branch, executes the decidim:upgrade task and creates a new branch and PR with those changes. Heroku is configured to create a Review App for each branch in that repo.

If I try to execute the task locally, what I get is:

npm ERR! code ENOENT
npm ERR! syscall open
npm ERR! path /Users/leonardodiez/Workspace/decidim-staging/tmp/npmbuild/decidim-webpacker-0.25.0-dev.tgz
npm ERR! errno -2
npm ERR! enoent ENOENT: no such file or directory, open '/Users/leonardodiez/Workspace/decidim-staging/tmp/npmbuild/decidim-webpacker-0.25.0-dev.tgz'
npm ERR! enoent This is related to npm not being able to find a file.
npm ERR! enoent 

If I remove the reference to the package located in tmp/npmbuild/... in my local machine it works properly, because it creates the tmp/npmbuil folder and then adds the package, but when I try to deploy it fails. The problem is that the tmp/npmbuild is created by the local installer package. I think this could work if we commit a package.json without decidim and let the deploy to install it for the first time every time. I'm trying it, but even if it works, we will have to upload an outdated package-lock.json file (as if you run npm i it will add the decidim dependency). And I think it will be very difficult to work with that approach.

UPDATE: it didn't work, it can't find the webpacker command.

[ahukkanen]

@leio10 Ah yeah, this is tricky. I think the NPM local cache has played tricks on me because it's fetching the packages from there if they don't exist at tmp/npmbuild.

When I remove the ~/.npm folder and re-run npm i in the staging app (the commit prior to removing the @decidim/x dependencies), I can replicate the error you are seeing.

I'll think about this if I can come up with a solution...

[ahukkanen]

@leio10 I think I have a solution for the problem above. It requires a small change in the deployment script, though, but does not require changing the package.json or package-lock.json.

So given this PR:
codegram/decidim-staging#354

First, revert commit e5539ec43509ff13de81240bc0f87f2211572679.

Then, change the deployment script to run this command before npm i:

$ npm exec -y --package github:decidim/decidim-npm-local -- decidimpack .

After that, install the dependencies as normal:

$ npm i

[ahukkanen]

@leio10 And at Heroku, I think you can achieve the above if you add this to the staging app's package.json:

  "scripts": {
    "heroku-prebuild": "npm exec -y --package github:decidim/decidim-npm-local -- decidimpack ."
  }

See: https://devcenter.heroku.com/articles/nodejs-support#heroku-specific-build-steps

[leio10]

@ahukkanen I tried your suggestion and it's almost working... but it seems that the contents of the .tgz files are not exactly the same, so I'm getting an error while checking the integrity of the file:

npm ERR! code EINTEGRITY
npm ERR! sha512-WZjonf/0FCMLQamdABgYBZW/stqXkgHGRlZv20z6jf4UoqKYzrTZyrgM9bnAydMPqwz71QvdY/uwb223eIhXLQ== integrity checksum failed when using sha512: wanted sha512-WZjonf/0FCMLQamdABgYBZW/stqXkgHGRlZv20z6jf4UoqKYzrTZyrgM9bnAydMPqwz71QvdY/uwb223eIhXLQ== but got sha512-spAK88SG6in8+9FvLAPI8LYQ1jQQIgT5ASO84UTowZLoZgYLMvW3eMr2O91daKA9CwYCZXcA9RZ3F3xyFaCDjQ==. (1410 bytes)

Would it be possible to remove the integrity check for this package somehow?

[ahukkanen]

@leio10 Do you mean you get that error at Heroku or locally?

I tried running the following locally in the staging app's PR branch:

$ npm run heroku-prebuild
$ npm ci

It worked fine.

If this happens at Heroku, I'm wondering what happens when bundle show decidim is run at Heroku. If that fails, it will fallback to the git packages which might cause an integrity error...

I'm also wondering which happens first at Heroku, does it install the NPM dependencies before running bundle install? If it is so, it would need to be changed the other way around.

[leio10]

@leio10 Do you mean you get that error at Heroku or locally?

I tried running the following locally in the staging app's PR branch:

$ npm run heroku-prebuild
$ npm ci

It worked fine.

If this happens at Heroku, I'm wondering what happens when bundle show decidim is run at Heroku. If that fails, it will fallback to the git packages which might cause an integrity error...

I'm also wondering which happens first at Heroku, does it install the NPM dependencies before running bundle install? If it is so, it would need to be changed the other way around.

Yes, definitely that's the problem. And it's worst, as it won't work if you install ruby before node :(

[ahukkanen]

it won't work if you install ruby before node :(

Yes, I was about to say that as I noticed it... 😄

Well, back to the drawing table.

[ahukkanen]

@leio10 I think I have a solution which requires this change in the local installer NPM package:
decidim-archive/decidim-npm-local#1

What that does is that if bundle show decidim fails, it will try to figure out the packages folder location using the project's Gemfile.lock. With the git installations, the Gemfile.lock has a reference to the correct git repository with the correct commit SHA, so if the bundle command is not available, it will fetch the code from git using the same commit hash.

This should make the SHA hashes match to avoid integrity violations.

[andreslucena]

I'm reviewing old PRs and found this one.

@ahukkanen does it make sense to retake this issue?

Personally, it'd be difficult to test this, as I don't have access to the Codegram's configured staging app, so I'd need to configure that and arrive to the same state that @leio10 got. Also, I'm afraid of touching webpacker configuration as it's really unstable. Maybe with jsbundling-rails/esbuild this isn't necessary, I don't know, you're my go-to guy for this NPM stuff 😄 .

How do you see it?

[ahukkanen]

@andreslucena Thanks for reminding.

I think the current approach is fine. I don't like is that we have to copy the packages to development apps / the design app, but I guess that's just something we have to live with. The approach taken in this PR is not perfect either, we were just trying to explore different ways to bypass the issue we had.

I feel that we should rely as long as possible with the defaults that NPM provides and avoid creating our own abstraction on top of it (that needs to be maintained). Let's just live with the limitations of NPM for now.

Closing this.