JackDotJS / vector-bot

A Discord bot for advanced moderation and server management.
https://discord.gg/s5nQBxFPp2
MIT License
15 stars 3 forks source link

Bump node-fetch from 2.6.2 to 3.1.1 #5

Closed dependabot[bot] closed 2 years ago

dependabot[bot] commented 2 years ago

Bumps node-fetch from 2.6.2 to 3.1.1.

Release notes

Sourced from node-fetch's releases.

v3.1.1

Security patch release

Recommended to upgrade, to not leak sensitive cookie and authentication header information to 3th party host while a redirect occurred

What's Changed

New Contributors

Full Changelog: https://github.com/node-fetch/node-fetch/compare/v3.1.0...v3.1.1

v3.1.0

What's Changed

... (truncated)

Changelog

Sourced from node-fetch's changelog.

Changelog

All notable changes will be recorded here.

The format is based on Keep a Changelog, and this project adheres to Semantic Versioning.

What's Changed

New Contributors

Full Changelog: https://github.com/node-fetch/node-fetch/compare/v3.1.0...v3.1.2

3.1.0

What's Changed

... (truncated)

Commits


Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) - `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language - `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language - `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language - `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/JackDotJS/vector-bot/network/alerts).
bdotsamir commented 2 years ago

:warning: WARNING! :warning: THIS WILL BREAK CJS REQUIRE STATEMENTS!

node-fetch v3 and up use ESM imports. There is no way to require them anymore.

JackDotJS commented 2 years ago

well that's just fuckin dandy

RobertRR11 commented 2 years ago

I'm using a kinda weird workaround from somewhere that makes it compatible with CJS

const fetch = (...args) => import('node-fetch').then(({ default: fetch }) => fetch(...args));

Not really great, but it works I guess

JackDotJS commented 2 years ago

can't we just use ESM imports for this one module?

import fetch from 'node-fetch'

i mean idk how the ESM thing works cus i havent had to use it until now

bdotsamir commented 2 years ago

No, you cant mix ESM imports with CJS requires. The closest you'll get is the dynamic import() statement which is kind of like require(), which is what @RobertRR11 uses (as hacky of a solution as it is). A project we'll have to work on further down the line is converting all require statements to ESM imports- although in PR #4, import syntax is the default with typescript ;)

JackDotJS commented 2 years ago

fucking uuuuuuuuuugggggggggggghhhhhhhhhhhhhhhhhhhhhhhhh whyyyyyyyyyyyyyyyyyy

JackDotJS commented 2 years ago

although in PR #4, import syntax is the default with typescript ;)

how does this work?

RobertRR11 commented 2 years ago

can't we just use ESM imports for this one module?

import fetch from 'node-fetch'

i mean idk how the ESM thing works cus i havent had to use it until now

As far as I know you can only use either ESM or CJS imports. But you can rename the extension of specific scripts to .mjs to make it ESM, but I don't know how well mixed ESM and CJS work together.

bdotsamir commented 2 years ago

although in PR #4, import syntax is the default with typescript ;)

how does this work?

What do you mean by that? how does import syntax work or why is it the default in typescript? I don't know the answer to that second question but I can answer the first one:

In ESM, each file no longer uses require() and module.exports. They're instead replaced by import x from 'y' and export respectively. You can see how it works in that PR, but I'll give you a quick rundown here:

file-one.ts

export default function helloWorld(): string {
  return "Hello world!";
}

export const foo = "bar";

file-two.ts

import hello from './file-one';
import { foo } from './file-one';
// or, an easier way to do it,
import hello, { foo } from './file-one';
// where "hello" is the default export and can be named anything when imported. think of `module.exports = function helloWorld()`.
// and "foo" is the *named* constant since it isnt default.

console.log(hello()); // Hello world!
console.log(foo); // bar

of course you can have both imports and exports in the same file if you want. another nifty feature is renaming named exports... i'll give you another example:

import { foo as theConstant } from './file-one';

console.log(theConstant); // bar

The benefit of ESM import syntax is that you're only importing the functions you need as opposed to importing the entire file and extracting out the functions you need. There's a difference between const { varOne, varTwo } = require('./file'); and import { varOne, varTwo } from './file'; in terms of the internals, where the latter only imports what you actually need.

bdotsamir commented 2 years ago

For what it's worth also, you can keep using node-fetch v2.x.x as long as you need. image

JackDotJS commented 2 years ago

ok well that makes sense... but can we use import syntax with the other 400 dependencies vector is using?

also for the record i dont really mind having to change the syntax for dependencies and stuff. ive seen ESM a few times before and it looks like its slowly becoming the standard as opposed to CJS, so i've been meaning to look into this for a while anyway.

bdotsamir commented 2 years ago

Fun fact: it's not currently possible to import a JSON file using import syntax, and mixing import and require in the same file is forbidden when "type": "module" is set in your package.json (which will infer each file ending in .js to be an es module, aka .mjs)

the only way to do this is to switch to typescript [winks harder] (or convert the json file to a JS object and just export the whole object)

https://stackoverflow.com/a/39855320

dependabot[bot] commented 2 years ago

OK, I won't notify you again about this release, but will get in touch when a new version is available. If you'd rather skip all updates until the next major or minor version, let me know by commenting @dependabot ignore this major version or @dependabot ignore this minor version.

If you change your mind, just re-open this PR and I'll resolve any conflicts on it.

JackDotJS commented 2 years ago

Fun fact: it's not currently possible to import a JSON file using import syntax

stressed-work