danger / danger-js

⚠️ Stop saying "you forgot to …" in code review
http://danger.systems/js/
MIT License
5.26k stars 368 forks source link

Allow parsing remote files as typescript #1366

Closed snowe2010 closed 1 year ago

snowe2010 commented 1 year ago

Currently, the transpiler.ts makes an assumption that the filename will always end with .ts, but this is incorrect in the case of remote dangerfiles that have refs attached, for example abc/123/dangerfile.ts?feature/branch Due to this, typescript parsing will always fail for remote files.

This commit adds a new parameter to the transpiler function, which parses the filename differently if the file is a remote one. This change also had to cascade up the stack to the runDangerfileEnvironment function, where we now accept a set of tuples, where the first is the filename, and the second is if it is remote or not. The only place the remote flag is being set currently is in the GitHub runner, where we download the remote dangerfile.

Finally, the pr subcommand failed to use the remote feature correctly, since the file would never exist when the command is first run. The check for the file has been removed, simply leaving behind the same check that is everywhere else.

Fixes: https://github.com/danger/danger-js/discussions/1359

orta commented 1 year ago

This makes sense WRT #1359 - when we used remote dangerfiles in Artsy it always was though Peril's runtime and so this flow largely would have been quick verifications from me for others. Great spot!

orta commented 1 year ago

Thanks, this is shipped as 11.2.4!

snowe2010 commented 1 year ago

Wow, I didn't expect a merge first try haha! Thank you very much!