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(compile): handle TypeScript file included as asset #27032

Merged

Conversation

dsherret
Copy link
Member

Closes #27024

Unverified

No user is associated with the committer email.

Unverified

No user is associated with the committer email.
@irbull
Copy link
Contributor

irbull commented Nov 23, 2024

I took this PR for a spin and it still seems to be failing with the example I posted in the issue. I'm running on Mac, so maybe that's related? Could also be a problem with how I built (I can't seem to verify after the build that ./deno was built from the right commit hash). However, I do have 6700ca8cc1b007a9aeae688fd31c75a86cd717c7 checked out and cargo clean && cargo build didn't produce any errors, so I'm fairly confident I have the right bits.

@irbull
Copy link
Contributor

irbull commented Nov 23, 2024

Actually, I can confirm that my custom built deno is in fact being used for deno compile, but I'm not sure that my custom built deno is being used as the runtime for the compiled artifact. Changes I made to cli/standalone/file_system do not seem to be reflected in the compiled executable. Would that make sense? Is there something I need to do during deno compile to ensure that I'm picking up the latest changes to the standalone runtime?

@dsherret
Copy link
Member Author

@irbull you need to set the DENORT_BIN env var (see #27015 (comment) for example)

@irbull
Copy link
Contributor

irbull commented Nov 23, 2024

Thanks David, that works! Google for DENORT_BIN produces no results, so I guess it's not documented. I've filed #27035 to add it to the docs. Feel free to add my face to that issue and I'll put together a PR.

As for this issue, with a a customer DenoRT, I can confirm that this PR addresses the issue. Furthermore, if you actually try and load the bundled .ts file you see the TypeScript (not the transpired JS). Good work.

@dsherret dsherret requested a review from bartlomieju November 25, 2024 01:33
@dsherret dsherret merged commit 4365f8d into denoland:main Nov 25, 2024
17 checks passed
@dsherret dsherret deleted the fix_compile_handle_ts_file_included_as_asset branch November 25, 2024 14:37
bartlomieju pushed a commit that referenced this pull request Nov 28, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Closes #27024
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.

Deno compile leads to TS errors if root directory is included
3 participants