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

fix: Include not working with deno runtime #2704

Open
wants to merge 8 commits into
base: 2.x
Choose a base branch
from

Conversation

stevefan1999-personal
Copy link

@stevefan1999-personal stevefan1999-personal commented Jul 29, 2024

Related to cdk8s-team/cdk8s#2171 but with a consequence of a modern version of Node must be required. This is one of the few roadblocks left.

Signed-off-by: Steve Fan <29133953+stevefan1999-personal@users.noreply.github.com>
Signed-off-by: Steve Fan <29133953+stevefan1999-personal@users.noreply.github.com>
Signed-off-by: Steve Fan <29133953+stevefan1999-personal@users.noreply.github.com>
Signed-off-by: Steve Fan <29133953+stevefan1999-personal@users.noreply.github.com>
@stevefan1999-personal stevefan1999-personal changed the title Change isolated loadurl process script to ESM feat: Change isolated loadurl process script to ESM Jul 29, 2024
@stevefan1999-personal
Copy link
Author

@iliapolo cc

@iliapolo
Copy link
Member

@stevefan1999-personal can you please elaborate on how this is related to Deno support? Also, you mentioned:

modern version of Node must be required

Which one?

@stevefan1999-personal
Copy link
Author

@stevefan1999-personal can you please elaborate on how this is related to Deno support? Also, you mentioned:

modern version of Node must be required

Which one?

Because Deno does not have a require function out of the box. And when you run that script you run with deno executable as well.

I don't remember starting from which Node you can use mjs as file extension running as ESM module exactly though.

@iliapolo iliapolo changed the title feat: Change isolated loadurl process script to ESM fix: Include not working with deno runtime Sep 9, 2024
Copy link
Member

@iliapolo iliapolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ESM has been available as a stable feature since Node12 so we are good here.

See https://nodejs.org/docs/latest-v12.x/api/esm.html

Thanks!

@iliapolo
Copy link
Member

iliapolo commented Sep 9, 2024

@stevefan1999-personal there's a self-mutation failure:

diff --git a/.projen/tasks.json b/.projen/tasks.json
index 665e4a0..8fb9228 [10](https://github.com/cdk8s-team/cdk8s-core/actions/runs/10772681725/job/29870857444?pr=2704#step:8:11)0644
--- a/.projen/tasks.json
+++ b/.projen/tasks.json
@@ -494,4 +494,4 @@
     "PATH": "$(npx -c \"node --print process.env.PATH\")"
   },
   "//": "~~ Generated by projen. To modify, edit .projenrc.js and run \"npx projen\"."
-}
\ No newline at end of file
+}

It means some auto-generated files have not been fully committed. You need to run yarn build locally and push the resulting changes.

@iliapolo iliapolo added the response-requested Awaiting response from author label Sep 9, 2024
@iliapolo
Copy link
Member

iliapolo commented Sep 9, 2024

@stevefan1999-personal

This is one of the few roadblocks left.

What other roadblocks are you aware of?

@github-actions github-actions bot removed the response-requested Awaiting response from author label Sep 9, 2024
@stevefan1999-personal
Copy link
Author

@stevefan1999-personal

This is one of the few roadblocks left.

What other roadblocks are you aware of?

There is a problem with the type checker. While it is not fatal it is annoying and hard for debugging

@iliapolo
Copy link
Member

@stevefan1999-personal

There is a problem with the type checker. While it is not fatal it is annoying and hard for debugging

Can you elaborate?

@iliapolo
Copy link
Member

@stevefan1999-personal

You need to run yarn build locally and push the resulting changes.

Reminder ^

@iliapolo iliapolo added the response-requested Awaiting response from author label Sep 10, 2024
Copy link

This PR has not received a response in a while and will be closed soon. If you want to keep it open, please leave a comment below @mentioning a maintainer.

@github-actions github-actions bot added the closing-soon Issue/PR will be closing soon if no response is provided label Oct 10, 2024
@pbar1
Copy link

pbar1 commented Oct 11, 2024

Any chance of revisiting this? I've also just run into this using Deno. @iliapolo

@github-actions github-actions bot removed closing-soon Issue/PR will be closing soon if no response is provided response-requested Awaiting response from author labels Oct 11, 2024
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 this pull request may close these issues.

3 participants