Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check-dist.yml fails consistently #339

Closed
chriscarrollsmith opened this issue Sep 27, 2023 · 4 comments
Closed

check-dist.yml fails consistently #339

chriscarrollsmith opened this issue Sep 27, 2023 · 4 comments
Assignees

Comments

@chriscarrollsmith
Copy link

chriscarrollsmith commented Sep 27, 2023

The check-dist.yml workflow has been a bit of a nightmare for me. Been trying to debug for over a day now. I run the build workflow locally (npm install + npm run bundle) and push the changes, but the remote build generated by check-dist.yml differs. The differences seem mostly related to webpack. Different module id numbers being assigned, for instance. I have tried forcing the runner to use the same OS (I am on Windows) and the same npm version, but that didn't fix it. I also tried disabling the npm cache and having the runner use 'npm install' rather than 'npm ci'. Same issue. Totally at a loss. Here are my workflow runs:

https://github.com/Promptly-Technologies-LLC/rss-fetch-action/actions/workflows/check-dist.yml

Maybe an ncc or webpack problem rather than a javascript-action problem, but I am probably going to have to entirely abandon this check-dist workflow in favor of building in the runner and then pushing to the repo.

@chriscarrollsmith
Copy link
Author

Closer analysis of the diffs reveals that not only index.js, but also licenses.txt is being created differently. The local build includes a license for something called safer-buffer and also substitutes an encoding license for the @vercel/ncc license generated in the cloud container. This definitely suggests the ncc build is doing something differently in the two environments, though I'm not sure why, given the use of the same lock file. Maybe something to do with different option defaults?

@chriscarrollsmith
Copy link
Author

Alright. Following the advice here, I was able to solve the problem by changing the "package" script from "ncc build src/index.js --license licenses.txt" to "ncc build src/index.js --license licenses.txt --external encoding". I honestly don't have a good enough understanding of ncc to know exactly what this flag is doing or why the default behavior was different on my local system than in the runner, but it wasn't an OS or ncc/npm/node version issue. I am guessing it's something weird I've got installed globally on my system that isn't installed on the runner. Anyway, maybe something to include in your documentation somewhere or even to make the default behavior in package.json.

@ncalteen
Copy link
Collaborator

ncalteen commented Oct 6, 2023

Hey @chriscarrollsmith thanks for pointing this out, as well as including your repo for reference. I'll take some time to look at this and see if there's anything we can do to prevent this on the template repo.

@ncalteen ncalteen self-assigned this Oct 6, 2023
@ncalteen
Copy link
Collaborator

I took some time on and off to look into this, and it appears there are a number of potential issues that would cause this error.

  • Line endings
    • I'll write up a PR for this to enforce this in .gitattribute.
  • Using npm ci vs npm install (though this appears to only have been an issue with older Node.js/npm versions)
    • This shouldn't be an issue if you're using the recommended Node version indicated in the README
  • Including source maps in the build
    • I ran into this one before and do recommend not including them. They are not included in the package.json scripts for this repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants