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

Add .devcontainer #334

Closed
yeikel opened this issue Apr 7, 2023 · 5 comments · Fixed by #337
Closed

Add .devcontainer #334

yeikel opened this issue Apr 7, 2023 · 5 comments · Fixed by #337

Comments

@yeikel
Copy link
Contributor

yeikel commented Apr 7, 2023

It'd be nice to have a .devcontainer in this project as the default devcontainer starts with a much higher version of Node and NPM

@jeffwidman
Copy link
Member

Apologies, I'm not very familiar with the Actions/Typescript/Node/NPM ecosystems...

Is there anything (other than time) holding us back from updating this to using the latest node/npm?

Obviously that means this problem will recur down the road, but this project doesn't typically see a lot of traffic, so if we start running into problems again it's probably a good reminder to update...

And this project is pretty simple so AFAIK we don't have a good reason to add a .devcontainer beyond pinning node/npm for dev purposes...

And to be clear, not asking you to handle that, you are certainly welcome to but if you haven't time we can get to it too at some point.

@yeikel
Copy link
Contributor Author

yeikel commented Apr 7, 2023

From what I understand, the limitation is in Github because the default runners are shipping with the Node 16 runtime

See actions/runner-images#5429 and https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#runs-for-javascript-actions

Also actions/runner-images#7002 (comment)

I am seeing mixed results from the threads, but it is probably safer to stick with 16 for a little longer (and more so given the differences between GHE and github.com)

To upgrade this action and use anything higher without any dependencies we would need to ask users to use an alternative container. This is technically doable (as it is a single line) but probably not desirable for everyone

@yeikel
Copy link
Contributor Author

yeikel commented Apr 7, 2023

I created a separate issue to track the potential upgrade #335

@jeffwidman
Copy link
Member

Hmm... those threads are confusing. Let me try to get clarity internally.

And yes, we do want to stick with using whatever the default node version is in the runner to make life easy for our users.

@jeffwidman jeffwidman changed the title Add .devcontainer Add .devcontainer Apr 7, 2023
@jeffwidman
Copy link
Member

Got clarity internally that node16 is the plan for now:

The runner ships with Node 12 and Node 16 to execute JavaScript actions. (With Node 12 being deprecated).

Separate from that, the hosted runner images will have a default version of Node - https://github.com/actions/runner-images

That's the version you'd get if you don't use actions/setup-node and you reference node from the PATH.

The runner does not use any arbitrary version of Node that's installed on the machine.

So as an action author, you should stick with Node 16 for the foreseeable future. (Matching what you specify in your action.yml)

So a .devcontainer.json is the best way forward to a pleasant dev experience for now. 👍

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

Successfully merging a pull request may close this issue.

2 participants