Skip to content

Commit

Permalink
Allow parsing remote files as typescript
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
snowe2010 committed Mar 2, 2023
1 parent f8dc6b5 commit 51bdc14
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 16 deletions.
4 changes: 1 addition & 3 deletions source/commands/danger-pr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import prettyjson from "prettyjson"
import { FakeCI } from "../ci_source/providers/Fake"
import { pullRequestParser } from "../platforms/pullRequestParser"
import { dangerfilePath } from "./utils/fileUtils"
import validateDangerfileExists from "./utils/validateDangerfileExists"
import setSharedArgs, { SharedCLI } from "./utils/sharedDangerfileArgs"
import { jsonDSLGenerator } from "../runner/dslGenerator"
import { prepareDangerDSL } from "./utils/runDangerSubprocess"
Expand Down Expand Up @@ -94,8 +93,7 @@ if (program.args.length === 0) {
const isJSON = app.js || app.json
const note = isJSON ? console.error : console.log
note(`Starting Danger PR on ${pr.repo}#${pr.pullRequestNumber}`)

if (customProcess || isJSON || validateDangerfileExists(dangerfilePath(program))) {
if (customProcess || isJSON || dangerfilePath(program)) {
if (!customProcess) {
d(`executing dangerfile at ${dangerfilePath(program)}`)
}
Expand Down
2 changes: 1 addition & 1 deletion source/commands/danger-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ const run = (config: SharedCLI) => async (jsonString: string) => {
if (platform.executeRuntimeEnvironment) {
await platform.executeRuntimeEnvironment(inline.runDangerfileEnvironment, dangerFile, runtimeEnv)
} else {
await inline.runDangerfileEnvironment([dangerFile], [undefined], runtimeEnv)
await inline.runDangerfileEnvironment([[dangerFile, false]], [undefined], runtimeEnv)
}
}

Expand Down
6 changes: 4 additions & 2 deletions source/platforms/GitHub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ const executeRuntimeEnvironment = async (
// dangerfile comes from the web, and do all the prefixing etc
let path: string
let content: string
let remote: boolean = false
if (existsSync(dangerfilePath)) {
path = dangerfilePath
content = readFileSync(dangerfilePath, "utf8")
Expand All @@ -142,8 +143,9 @@ const executeRuntimeEnvironment = async (
// Chop off the danger import
const newDangerfile = cleanDangerfile(dangerfileContent)

remote = true
// Cool, transpile it into something we can run
content = transpiler(newDangerfile, dangerfilePath)
content = transpiler(newDangerfile, dangerfilePath, remote)
}

// If there's an event.json - we should always pass it inside the default export
Expand All @@ -155,7 +157,7 @@ const executeRuntimeEnvironment = async (
}

// Actually start up[ the runtime evaluation
await start([path], [content], environment, defaultExport)
await start([[path, remote]], [content], environment, defaultExport)

// Undo the runtime
restoreOriginalModuleLoader()
Expand Down
2 changes: 1 addition & 1 deletion source/runner/Executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export class Executor {
// If an eval of the Dangerfile fails, we should generate a
// message that can go back to the CI
try {
results = await this.runner.runDangerfileEnvironment([file], [undefined], runtime)
results = await this.runner.runDangerfileEnvironment([[file, false]], [undefined], runtime)
} catch (error) {
results = this.resultsForError(error as Error)
}
Expand Down
15 changes: 10 additions & 5 deletions source/runner/runners/inline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ const runAllScheduledTasks = async (results: DangerRuntimeContainer) => {
* @returns {DangerResults} the results of the run
*/
export const runDangerfileEnvironment = async (
filenames: string[],
filenames: [string, boolean][],
originalContents: (string | undefined)[] | undefined,
environment: DangerContext,
injectedObjectToExport?: any,
Expand Down Expand Up @@ -84,11 +84,16 @@ export const runDangerfileEnvironment = async (
// Loop through all files and their potential contents, they edit
// results inside the env, so no need to keep track ourselves

for (const filename of filenames) {
const index = filenames.indexOf(filename)
const originalContent = (originalContents && originalContents[index]) || fs.readFileSync(filename, "utf8")
for (let index = 0; index < filenames.length; index++) {
const [filename, remote] = filenames[index]
let fn: string = filename
if (remote) {
d(`File ${filename} is a remote dangerfile`)
fn = filename.split("@")[0]
}
const originalContent = (originalContents && originalContents[index]) || fs.readFileSync(fn, "utf8")
let content = cleanDangerfile(originalContent)
let compiled = compile(content, filename)
let compiled = compile(content, filename, remote)

try {
// Move all the DSL attributes into the global scope
Expand Down
5 changes: 3 additions & 2 deletions source/runner/runners/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ export interface DangerRunner {
* Executes a Dangerfile at a specific path, with a context.
* The values inside a Danger context are applied as globals to the Dangerfiles runtime.
*
* @param {string[]} filenames a set of file paths for the dangerfile
* @param {[string, boolean][]} filenames a set of tuples of file paths for the dangerfile, with a boolean indicating
* if it is a remote path
* @param {string[] | undefined[]} originalContents optional, the JS pre-compiled
* @param {DangerContext} environment the results of createDangerfileRuntimeEnvironment
* @param {any | undefined} injectedObjectToExport an optional object for passing into default exports
* @returns {DangerResults} the results of the run
*/
runDangerfileEnvironment: (
filenames: string[],
filenames: [string, boolean][],
originalContents: string[] | undefined[] | undefined,
environment: any,
injectedObjectToExport?: any
Expand Down
14 changes: 12 additions & 2 deletions source/runner/runners/utils/transpiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,23 +183,33 @@ export const babelify = (content: string, filename: string, extraPlugins: string
return result.code
}

export default (code: string, filename: string) => {
export default (code: string, filename: string, remoteFile: boolean = false) => {
if (!hasChecked) {
checkForNodeModules()
}

const filetype = path.extname(filename)
let filetype: string
if (remoteFile) {
d(`Parsing the file from the remote reference ${filename}`)
let [file, _] = filename.split("@")
filetype = path.extname(file)
} else {
filetype = path.extname(filename)
}
const isModule = filename.includes("node_modules")
if (isModule) {
return code
}

let result = code
if (hasNativeTypeScript && filetype.startsWith(".ts")) {
d("compiling with typescript")
result = typescriptify(code, path.dirname(filename))
} else if (hasBabel && hasBabelTypeScript && filetype.startsWith(".ts")) {
d("compiling as typescript with babel")
result = babelify(code, filename, [`${babelPackagePrefix}plugin-transform-typescript`])
} else if (hasBabel && filetype.startsWith(".js")) {
d("babelifying as javascript")
result = babelify(code, filename, hasFlow ? [`${babelPackagePrefix}plugin-transform-flow-strip-types`] : [])
}

Expand Down

0 comments on commit 51bdc14

Please sign in to comment.