Automattic / wp-api-console

WordPress (.com and .org) API Console written in React/Redux
GNU General Public License v2.0
70 stars 20 forks source link

Introduce package lock to prevent future breakage on dependency changes #92

Closed dmsnell closed 2 years ago

dmsnell commented 2 years ago

In #89 we had to make a fix to restore the ability to build this project because a dependency started shipping untranspiled code. This could have been prevented if we had been using a package-lock.json file and retained the old dependency version.

In this patch we're introducing a package lock based on the current project dependencies to prevent such a future problem from appearing, inasmuch as we are able to control the parocess.

Props to @youknowriad for suggesting this in #89

Testing

Should be fine to build this locally and verify that it works. No need to re-deploy after merging this because it won't/shouldn't change the build artifacts (unless sufficient "time" passes between now and then).

sirreal commented 2 years ago

I get an old lockfile warning with this, I'm using npm@8. Is there a specific npm version we should target?

dmsnell commented 2 years ago

I get an old lockfile warning with this, I'm using npm@8. Is there a specific npm version we should target?

Thanks, I should have noted that I'm having issues with node versions. Thinking of pinning node12 in this PR too as that seems to work for me, but I think I made this lock file with node16

sirreal commented 2 years ago

Node 12 is end-of-life in April (2022-04-30), that's only about a month from now.

Node 14 (maintenance LTS for ~13 months) might be better if we want to support an older version.

(From the official releases page)

Node 14 does ship with npm 6, which I believe uses the old lockfile version.

Node 16 is the current LTS release and it ships with npm 8. I recall that npm changed the lockfile with npm v7.

It seems fine to use either version of the lockfile.

dmsnell commented 2 years ago

@sirreal it does seem like it was the newer npm that made things crash (I think; I hope). I've regenerated this using the latest v16 of node and v6 of nom and it seems fine.

~Needs to wait on #93 to merge since you can't technically build this branch until that fix is in.~