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

Error with importing Danger module in Dangerfile.js #1459

Open
npatmedia opened this issue Jul 25, 2024 · 5 comments
Open

Error with importing Danger module in Dangerfile.js #1459

npatmedia opened this issue Jul 25, 2024 · 5 comments
Labels

Comments

@npatmedia
Copy link

I'm encountering an error when trying to import the Danger module in my Dangerfile.js. Additionally, the provided Spectrum discussion thread link (https://spectrum.chat/?t=0a005b56-31ec-4919-9a28-ced623949d4d) is broken and returns a 404 error.

Error Message:
throw "\nHey there, it looks like you're trying to import the danger module. Turns out\nthat the code you write in a Dangerfile.js is actually a bit of a sneaky hack. \n\nWhen running Danger, the import or require for Danger is removed before the code\nis evaluated. Instead all of the imports are added to the global runtime, so if\nyou are importing Danger to use one of it's functions - you should instead just\nuse the global object for the root DSL elements.\n\nThere is a spectrum thread for discussion here:\n - https://spectrum.chat/?t=0a005b56-31ec-4919-9a28-ced623949d4d\n";
^
Hey there, it looks like you're trying to import the danger module. Turns out
that the code you write in a Dangerfile.js is actually a bit of a sneaky hack.
When running Danger, the import or require for Danger is removed before the code
is evaluated. Instead all of the imports are added to the global runtime, so if
you are importing Danger to use one of it's functions - you should instead just
use the global object for the root DSL elements.
There is a spectrum thread for discussion here:

Steps to Reproduce:

  1. Create a Dangerfile.js with the following content:

`import { danger, markdown, message, warn } from 'danger';

// Example usage of danger
markdown('This is a markdown message');`

  1. Run the Danger.js script using Node.js v20.11.0.
    Expected Behavior:
    The Danger.js script should run without throwing an import error, and the appropriate actions should be executed as specified in the Dangerfile.js.

Actual Behavior:
The script throws an error indicating that importing the Danger module is not allowed. Additionally, the provided link to the Spectrum discussion thread is broken and returns a 404 error.

Environment:

Node.js version: v20.11.0
Danger.js version: [Specify the version you are using]
Additional Context:
The error suggests that the import for Danger is removed before the code is evaluated, and all imports are added to the global runtime instead. However, there is no clear documentation on this behavior, and the broken Spectrum link makes it difficult to find further information or discussion on the topic.

Please provide guidance on how to properly use Danger.js with Node.js v20.11.0 and any updates on the broken Spectrum thread link.

@npatmedia npatmedia added the bug label Jul 25, 2024
@orta
Copy link
Member

orta commented Jul 25, 2024

I assume then you don't have babel or typescript in your repo which would remove those imports I think, they are largely ceremonial and only exist to ensure a reasonable dev experience - they are actually globals, so you're OK to just write markdown or danger in the .js file 👍🏻

@fbartho
Copy link
Member

fbartho commented Jul 25, 2024

I confirm @orta’s statements, in JS-alone, you can either remove the import statement, or convert it to a require

To elaborate: When I last tweaked the relevant code in DangerJS, import wasn’t a keyword for JS-only codebases. There’s probably work to be done around DangerJS usage with JS & ESM-imports in node20.

As you mentioned, we definitely should update things to no longer reference the Spectrum link, as Spectrum is defunct for us.

@fbartho
Copy link
Member

fbartho commented Jul 25, 2024

This file is supposed to strip the import out:
https://github.com/danger/danger-js/blob/92d2525fe338bff16ae7d42794d0a835e2d27473/source/runner/runners/utils/cleanDangerfile.ts

Some relevant call sites:

let content = cleanDangerfile(originalContent)

const newDangerfile = cleanDangerfile(dangerfileContent)

const newDangerfile = cleanDangerfile(dangerfileContent)

@fbartho
Copy link
Member

fbartho commented Jul 25, 2024

One question: you’re not just trying to node Dangerfile.js or otherwise execute the dangerfile that way, right?
How are you triggering danger?

(Danger has a CLI tool that you use to trigger it that ensures all the environmental conditions are right for your code to be executed correctly!)

@npatmedia
Copy link
Author

sorry for late response.
we use it with ts, SWC and webpack

it logs like
/home/runner/work/webmobile-pwa/webmobile-pwa/node_modules/.pnpm/danger@12.3.3/node_modules/danger/distribution/danger.js:3
throw "\nHey there, it looks like you're trying to import the danger module. Turns out\nthat the code you write in a Dangerfile.js is actually a bit of a sneaky hack. \n\nWhen running Danger, the import or require for Danger is removed before the code\nis evaluated. Instead all of the imports are added to the global runtime, so if\nyou are importing Danger to use one of it's functions - you should instead just\nuse the global object for the root DSL elements.\n\nThere is a spectrum thread for discussion here:\n - https://spectrum.chat/?t=0a005b56-31ec-4919-9a28-ced623949d4d\n";
^

Hey there, it looks like you're trying to import the danger module. Turns out
that the code you write in a Dangerfile.js is actually a bit of a sneaky hack.

When running Danger, the import or require for Danger is removed before the code
is evaluated. Instead all of the imports are added to the global runtime, so if
you are importing Danger to use one of it's functions - you should instead just
use the global object for the root DSL elements.

There is a spectrum thread for discussion here:

(Use node --trace-uncaught ... to show where the exception was thrown)

Node.js v20.11.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants