How to update the build - dist/*.js files

[dorny]

@dharmendrasha I would really appreciate your opinion on this.

We are packaging all .js code into dist/index.js - as it's done in the typescript-action template. The reasoning is to make the action immediately runnable, without waiting for npm i.

This has two unfortunate consequences:

• We are mixing the code repository with the build artifacts storage. The builds now have ~2.8MB. Every change to those files adds them to git history so if we changed them after every PR, maybe a few times in the PR, the repository size will grow. This is probably not a concern for the usage of the action - I guess when the action is checked out in a workflow, it fetches only the single specific commit.
• dist/index.js file is too big to be manually reviewed. It wouldn't be hard to inject malicious code into this action hidden in otherwise relevant PR.

Up until now, I updated the dist/*.js files only before releasing a new version instead of every PR and also I avoided merging changes to the dist/index.js made by others. With more PRs coming this is not sustainable.

What would you suggest?
I was thinking about documenting contribution guidelines where it would be stated the dist/index.js should not be modified in the PRs and instead automate the build as part of the workflow run. The best would be if we could fail the check run if the file is modified and at the same time the workflow would run the build and update the file automatically before the PR is merged or before the release.

[flobernd]

@dorny The upstream typescript-action template repository contains a workflow that generates the "dist" redistributable files on merge/push and compares the result to the checked in file. I would recommend to copy that workflow :-)

This prevents that the generated index.js ever differs from the checked in one and basically completely solves point 2.

For point 1 there is not much you can do. But a few MiB per commit should not hurt in general. GitHub will do shallow clones for the actions as you correctly assumed. Squash merging the PRs could help a little bit, if appropriate.

[dorny]

@flobernd thanks for the suggestion. I see they added it a year ago. I guess for the same reason I've just mentioned. IIRC it wasn't there before and when I tried to do something like that in the beginning, I've run into issues with ncc, It produced slightly different output on Windows and Linux. The difference was not only in the line endings so the check was failing.

[flobernd]

@dorny I encountered this issue as well, but there is an easy fix for that. First of all we need a .gitattributes file that forces the line endings to LF for .js files (or at least all files in the "dist" folder).

Besides that, source map generation needs to be disabled (just remove the argument passed to ncc), because this one contains different stuff on Windows vs. Linux. Source map generation for release builds is not useful anyways, so there won't be any bigger negative side-effects.

[dorny]

@flobernd Turns out we already had .gitattributes set to force LF. I also don't see any absolute path from my own PC in the source maps. There must have been some other issue that was resolved in the last two years. I'm adding the check from the typescript-action template.

Locally when you have index.js checked out with LF endings and run npm run package you will see the files as modified even when the CRLF would get converted to the LF on the remote. I added a package to convert CRLF to LF in the dist folder as part of the package command. This should prevent it.

[dorny]

Interestingly, the content of the source map was the same on windows and WSL but the diff was detected when it was running in GitHub action. I'm disabling the source-map, it's not much useful as you said.