Fix slow build issue

[snowteamer]

Summary of changes

• Should restore the main build time to its usual values from commit b1f60db.

Additional information

The lockfile in this PR has been patched by doing the following:

git checkout master
git restore package-lock.json --source b1f60dbcafdbbf801b4b7c14cdf7ce681a39cce6
npm i
rm ./package-lock.json
npm i

If testing locally, please make sure to do npm ci once rather than another npm i before grunt commands, to avoid the lockfile being altered.

[socket-security]

New, updated, and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher

🚮 Removed packages: npm/[email protected], npm/[email protected], npm/[email protected]

View full report↗︎

[cypress]

group-income    Run #3791

Run Properties:   Passed #3791  •  3cddca509b ℹ️: Merge 9bc1c2f740301e84838f48f9d026c86ea11c3456 into dae6ad647324c4ae269a5d291479...

Project	group-income
Branch Review	fix-slow-build-issue
Run status	Passed #3791
Run duration	10m 52s
Commit	3cddca509b ℹ️: Merge 9bc1c2f740301e84838f48f9d026c86ea11c3456 into dae6ad647324c4ae269a5d291479...
Committer	Snowteamer
View all properties for this run ↗︎

---

Test results
Failures	0
Flaky	0
Pending	10
Skipped	0
Passing	112
View all changes introduced in this branch ↗︎

[taoeffect]

Nice work @snowteamer tracking this down!

Left one question for you + some questions here:

• Is there any way to track down the specific dependency that is causing this and only alter that?
• Regarding the instructions, I don't quite understand them, because they both say to run npm i and not to run npm i:

"To patch lockfile, run [...] npm i"
[..]
If testing locally, please make sure to do npm ci once rather than npm i before grunt commands, to avoid the lockfile being altered.

Can you explain?

(What does npm ci do?)

• And what happens if you delete the package-lock.json file and create it afresh? Ideally we'd want the latest of everything, with the minimal number of changes to it.

b1f60db is really far back in the history, so I'd like to leave as many of the dependencies at their later version as possible, but this PR makes over 12k changes to the package-lock.json file, making it hard to review and figure out what's going on.

[taoeffect]

Why was this commented out? 🤔

(I think @corrideat added this for some specific reason)

[snowteamer]

It caused a syntax error, complaining about function (this) being invalid code, like if only the : Object part had been removed when transpiling JS with Flow annotations to plain JS.

[corrideat]

@snowteamer What complained about a syntax error? It's definitely a JavaScript syntax error (this isn't allowed as an identifier), but it's not a Flow (or TypeScript syntax error), and this annotation is used to represent the type of this.

For example:

type Foo = {
  bar: number
}

function usesFoo(this: Foo) {
  // Ok, because we've declared `this` should be of type `Foo`
  alert(this.bar)
}

function usesFoo() {
  // Not ok, because this is assumed to be `window` / `self` / `globalThis`,
  // and `.bar` is an invalid property of the global object.
  alert(this.bar)
}

[corrideat]

Replying to myself, if the error is Flow-related, it could be because packages such as @babel/plugin-syntax-flow are being downgraded with this PR, and the older version maybe didn't have support for these this annotations.

[snowteamer]

@taoeffect

Is there any way to track down the specific dependency that is causing this and only alter that?

I would like to. For now I don't know.

The first npm i is only part of the procedure I've used to patch the lockfile.

That old lockfile specified some different dependencies, so I've used npm i to have npm somehow use the current package.json file to satisfy our current dependencies, while using the old lockfile - rather than the current one - as a kind of starting point regarding the subdeps.
Accordingly, this modified the lockfile again, which was now different from the master lockfile, while satisfying the master package file.
There are many possible lockfiles which are compatible with a given package file. The lockfile from master is one, and this PR's one is just another. On my end it appears to solve the issue.

On the other hand, npm ci checks whether the lockfile and the package file are compatible then delete the node_modules folder and reinstalls everything specified in the lockfile.
This way the old local node_modules contents don't affect the outcome, which I'm not sure is true when doing npm i.

And what happens if you delete the package-lock.json file and create it afresh?

rm ./package-lock.json && npm i doesn't seem to resurrect the slowness issue, and removes a bunch of esbuild platform-dependants modules as well as an fsevents version in the lockfile. Nothing seems to be added or updated, so I'll update the PR accordingly.

making it hard to review and figure out what's going on.

That's why I've shared how to generate the file, rather than just submitting it. Granted, this still doesn't make clear which node modules have changed though.

[taoeffect]

@snowteamer based on everything you've said I decided to follow your instructions and re-created the package-lock.json file locally.

Perhaps because I was the one who created it originally, my recreation of it appears to have resulted in a significantly smaller set of changes to the file. I'm guessing it's because the file is OS-dependent.

In my tests this does seem to have fixed the issue, and it didn't require changes to the source either.

So I've pushed these changes here and have credited you:

4b8263d

Could you please try pulling master locally and let me know if the new lock file fixes the build times for you too?

[snowteamer]

@taoeffect
Unfortunately, after pulling in the change and doing npm ci && grunt dev, the build was slow again.

Did you do npm ci to apply the lockfile after recreating it on your end? Otherwise, maybe you still had an old node_modules folder from a previous test where it was fast

[taoeffect]

@snowteamer I deleted my entire node_modules and ran npm i ... just pushed e9f4ce3

Does it work for you?