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

Switch back to third party fetch lib for all node versions #2961

Merged
merged 4 commits into from
Oct 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ Special thanks to: @rejas, @sdetweil

### Removed

- Removed usage of internal fetch function of node until it is more stable.

### Updated

- Cleaned up test directory (#2937) and jest config (#2959)
Expand Down
28 changes: 18 additions & 10 deletions js/fetch.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,28 @@
/**
* fetch
* Helper class to provide either third party fetch library or (if node >= 18)
* return internal node fetch implementation.
*
* Attention: After some discussion we always return the third party
* implementation until the node implementation is stable and more tested
*
* @see https://github.com/MichMich/MagicMirror/pull/2952
* @see https://github.com/MichMich/MagicMirror/issues/2649
* @param {string} url to be fetched
* @param {object} options object e.g. for headers
* @class
*/
async function fetch(url, options = {}) {
const nodeVersion = process.version.match(/^v(\d+)\.*/)[1];
if (nodeVersion >= 18) {
// node version >= 18
return global.fetch(url, options);
} else {
// node version < 18
const nodefetch = require("node-fetch");
return nodefetch(url, options);
}
// const nodeVersion = process.version.match(/^v(\d+)\.*/)[1];
// if (nodeVersion >= 18) {
// // node version >= 18
// return global.fetch(url, options);
// } else {
// // node version < 18
// const nodefetch = require("node-fetch");
// return nodefetch(url, options);
// }
const nodefetch = require("node-fetch");
return nodefetch(url, options);
}

module.exports = fetch;
10 changes: 8 additions & 2 deletions tests/e2e/helpers/global-setup.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const jsdom = require("jsdom");
const corefetch = require("fetch");

exports.startApplication = async (configFilename, exec) => {
jest.resetModules();
Expand Down Expand Up @@ -82,8 +83,13 @@ exports.waitForAllElements = (selector) => {
});
};

// When native fetch is used keep-alive is set which causes issues with tests that should not share the connection, fall back to use the older one for now...
exports.fetch = require("node-fetch");
exports.fetch = (url) => {
return new Promise((resolve) => {
corefetch(url).then((res) => {
resolve(res);
});
});
};

exports.testMatch = async (element, regex) => {
const elem = await this.waitForElement(element);
Expand Down