From d5e855dd6df6380fcaca9fb28580e4bfb14ecd2d Mon Sep 17 00:00:00 2001 From: Veeck Date: Thu, 6 Oct 2022 19:44:16 +0200 Subject: [PATCH] Use fetch instead of XMLHttpRequest in weatherprovider (#2935) small update to the fetchData method to use the fetch helper instead of the old XCMLHttpRequest. Also fixes some typos :-) Co-authored-by: veeck --- CHANGELOG.md | 2 + js/fetch.js | 2 +- .../default/weather/providers/envcanada.js | 10 ++-- .../weather/providers/openweathermap.js | 8 ++- modules/default/weather/providers/smhi.js | 6 +-- .../weather/providers/ukmetofficedatahub.js | 14 ++--- modules/default/weather/weatherobject.js | 6 +-- modules/default/weather/weatherprovider.js | 51 ++++++++----------- 8 files changed, 45 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c6fa86d..208ece56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,8 @@ Special thanks to: @rejas, @sdetweil - Updated e2e tests (moved `done()` in helper functions) and use es6 syntax in all tests - Updated da translation +- Rework weather module + - Use fetch instead of XMLHttpRequest in weatherprovider ### Fixed diff --git a/js/fetch.js b/js/fetch.js index fb855f2a..8009dc16 100644 --- a/js/fetch.js +++ b/js/fetch.js @@ -5,7 +5,7 @@ * @param {object} options object e.g. for headers * @class */ -async function fetch(url, options) { +async function fetch(url, options = {}) { const nodeVersion = process.version.match(/^v(\d+)\.*/)[1]; if (nodeVersion >= 18) { // node version >= 18 diff --git a/modules/default/weather/providers/envcanada.js b/modules/default/weather/providers/envcanada.js index 589cec6e..a903fd04 100644 --- a/modules/default/weather/providers/envcanada.js +++ b/modules/default/weather/providers/envcanada.js @@ -74,7 +74,7 @@ WeatherProvider.register("envcanada", { // Override the fetchCurrentWeather method to query EC and construct a Current weather object // fetchCurrentWeather() { - this.fetchData(this.getUrl(), "GET", "xml") + this.fetchData(this.getUrl(), "xml") .then((data) => { if (!data) { // Did not receive usable new data. @@ -94,7 +94,7 @@ WeatherProvider.register("envcanada", { // Override the fetchWeatherForecast method to query EC and construct Forecast weather objects // fetchWeatherForecast() { - this.fetchData(this.getUrl(), "GET", "xml") + this.fetchData(this.getUrl(), "xml") .then((data) => { if (!data) { // Did not receive usable new data. @@ -114,7 +114,7 @@ WeatherProvider.register("envcanada", { // Override the fetchWeatherHourly method to query EC and construct Forecast weather objects // fetchWeatherHourly() { - this.fetchData(this.getUrl(), "GET", "xml") + this.fetchData(this.getUrl(), "xml") .then((data) => { if (!data) { // Did not receive usable new data. @@ -137,8 +137,8 @@ WeatherProvider.register("envcanada", { ////////////////////////////////////////////////////////////////////////////////// // - // Build the EC URL based on the Site Code and Province Code specified in the config parms. Note that the - // URL defaults to the Englsih version simply because there is no language dependancy in the data + // Build the EC URL based on the Site Code and Province Code specified in the config params. Note that the + // URL defaults to the English version simply because there is no language dependency in the data // being accessed. This is only pertinent when using the EC data elements that contain a textual forecast. // getUrl() { diff --git a/modules/default/weather/providers/openweathermap.js b/modules/default/weather/providers/openweathermap.js index b0f5eedf..6ba73c2f 100644 --- a/modules/default/weather/providers/openweathermap.js +++ b/modules/default/weather/providers/openweathermap.js @@ -21,7 +21,7 @@ WeatherProvider.register("openweathermap", { weatherEndpoint: "", // can be "onecall", "forecast" or "weather" (for current) locationID: false, location: false, - lat: 0, // the onecall endpoint needs lat / lon values, it doesn'T support the locationId + lat: 0, // the onecall endpoint needs lat / lon values, it doesn't support the locationId lon: 0, apiKey: "" }, @@ -147,8 +147,7 @@ WeatherProvider.register("openweathermap", { return this.fetchForecastDaily(forecasts); } // if weatherEndpoint does not match forecast or forecast/daily, what should be returned? - const days = [new WeatherObject(this.config.units, this.config.tempUnits, this.config.windUnits, this.config.useKmh)]; - return days; + return [new WeatherObject(this.config.units, this.config.tempUnits, this.config.windUnits, this.config.useKmh)]; }, /* @@ -159,8 +158,7 @@ WeatherProvider.register("openweathermap", { return this.fetchOnecall(data); } // if weatherEndpoint does not match onecall, what should be returned? - const weatherData = { current: new WeatherObject(this.config.units, this.config.tempUnits, this.config.windUnits, this.config.useKmh), hours: [], days: [] }; - return weatherData; + return { current: new WeatherObject(this.config.units, this.config.tempUnits, this.config.windUnits, this.config.useKmh), hours: [], days: [] }; }, /* diff --git a/modules/default/weather/providers/smhi.js b/modules/default/weather/providers/smhi.js index c42b460b..d3ff79ac 100644 --- a/modules/default/weather/providers/smhi.js +++ b/modules/default/weather/providers/smhi.js @@ -174,7 +174,7 @@ WeatherProvider.register("smhi", { }, /** - * Takes all of the data points and converts it to one WeatherObject per day. + * Takes all the data points and converts it to one WeatherObject per day. * * @param {object[]} allWeatherData Array of weatherdata * @param {object} coordinates Coordinates of the locations of the weather @@ -203,7 +203,7 @@ WeatherProvider.register("smhi", { result.push(currentWeather); } - //Keep track of what icons has been used for each hour of daytime and use the middle one for the forecast + //Keep track of what icons have been used for each hour of daytime and use the middle one for the forecast if (weatherObject.isDayTime()) { dayWeatherTypes.push(weatherObject.weatherType); } @@ -271,7 +271,7 @@ WeatherProvider.register("smhi", { /** * Map the icon value from SMHI to an icon that MagicMirror² understands. - * Uses different icons depending if its daytime or nighttime. + * Uses different icons depending on if its daytime or nighttime. * SMHI's description of what the numeric value means is the comment after the case. * * @param {number} input The SMHI icon value diff --git a/modules/default/weather/providers/ukmetofficedatahub.js b/modules/default/weather/providers/ukmetofficedatahub.js index 216f6619..508d700d 100644 --- a/modules/default/weather/providers/ukmetofficedatahub.js +++ b/modules/default/weather/providers/ukmetofficedatahub.js @@ -89,7 +89,7 @@ WeatherProvider.register("ukmetofficedatahub", { fetchCurrentWeather() { this.fetchWeather(this.getUrl("hourly"), this.getHeaders()) .then((data) => { - // Check data is useable + // Check data is usable if (!data || !data.features || !data.features[0].properties || !data.features[0].properties.timeSeries || data.features[0].properties.timeSeries.length === 0) { // Did not receive usable new data. // Maybe this needs a better check? @@ -109,7 +109,7 @@ WeatherProvider.register("ukmetofficedatahub", { // Catch any error(s) .catch((error) => Log.error("Could not load data: " + error.message)) - // Let the module know there're new data available + // Let the module know there is data available .finally(() => this.updateAvailable()); }, @@ -140,7 +140,7 @@ WeatherProvider.register("ukmetofficedatahub", { currentWeather.precipitation = forecastDataHours[hour].probOfPrecipitation; currentWeather.feelsLikeTemp = this.convertTemp(forecastDataHours[hour].feelsLikeTemperature); - // Pass on full details so they can be used in custom templates + // Pass on full details, so they can be used in custom templates // Note the units of the supplied data when using this (see top of file) currentWeather.rawData = forecastDataHours[hour]; } @@ -148,7 +148,7 @@ WeatherProvider.register("ukmetofficedatahub", { // Determine the sunrise/sunset times - (still) not supplied in UK Met Office data // Passes {longitude, latitude} to SunCalc, could pass height to, but - // SunCalc.getTimes doesnt take that into account + // SunCalc.getTimes doesn't take that into account currentWeather.updateSunTime(this.config.lat, this.config.lon); return currentWeather; @@ -158,7 +158,7 @@ WeatherProvider.register("ukmetofficedatahub", { fetchWeatherForecast() { this.fetchWeather(this.getUrl("daily"), this.getHeaders()) .then((data) => { - // Check data is useable + // Check data is usable if (!data || !data.features || !data.features[0].properties || !data.features[0].properties.timeSeries || data.features[0].properties.timeSeries.length === 0) { // Did not receive usable new data. // Maybe this needs a better check? @@ -178,7 +178,7 @@ WeatherProvider.register("ukmetofficedatahub", { // Catch any error(s) .catch((error) => Log.error("Could not load data: " + error.message)) - // Let the module know there're new data available + // Let the module know there is new data available .finally(() => this.updateAvailable()); }, @@ -216,7 +216,7 @@ WeatherProvider.register("ukmetofficedatahub", { forecastWeather.snow = forecastDataDays[day].dayProbabilityOfSnow; forecastWeather.feelsLikeTemp = this.convertTemp(forecastDataDays[day].dayMaxFeelsLikeTemp); - // Pass on full details so they can be used in custom templates + // Pass on full details, so they can be used in custom templates // Note the units of the supplied data when using this (see top of file) forecastWeather.rawData = forecastDataDays[day]; diff --git a/modules/default/weather/weatherobject.js b/modules/default/weather/weatherobject.js index c2e6727b..14eca49f 100644 --- a/modules/default/weather/weatherobject.js +++ b/modules/default/weather/weatherobject.js @@ -134,15 +134,15 @@ class WeatherObject { /** * Update the sunrise / sunset time depending on the location. This can be - * used if your provider doesnt provide that data by itself. Then SunCalc + * used if your provider doesn't provide that data by itself. Then SunCalc * is used here to calculate them according to the location. * * @param {number} lat latitude * @param {number} lon longitude */ updateSunTime(lat, lon) { - let now = !this.date ? new Date() : this.date.toDate(); - let times = SunCalc.getTimes(now, lat, lon); + const now = !this.date ? new Date() : this.date.toDate(); + const times = SunCalc.getTimes(now, lat, lon); this.sunrise = moment(times.sunrise, "X"); this.sunset = moment(times.sunset, "X"); } diff --git a/modules/default/weather/weatherprovider.js b/modules/default/weather/weatherprovider.js index d24623b8..bf1deee2 100644 --- a/modules/default/weather/weatherprovider.js +++ b/modules/default/weather/weatherprovider.js @@ -119,37 +119,28 @@ const WeatherProvider = Class.extend({ } }, - // A convenience function to make requests. It returns a promise. - fetchData: function (url, method = "GET", type = "json") { + /** + * A convenience function to make requests. + * + * @param {string} url the url to fetch from + * @param {string} type what contenttype to expect in the response, can be "json" or "xml" + * @returns {Promise} resolved when the fetch is done + */ + fetchData: async function (url, type = "json") { url = this.getCorsUrl() + url; - const getData = function (mockData) { - return new Promise(function (resolve, reject) { - if (mockData) { - let data = mockData; - data = data.substring(1, data.length - 1); - resolve(JSON.parse(data)); - } else { - const request = new XMLHttpRequest(); - request.open(method, url, true); - request.onreadystatechange = function () { - if (this.readyState === 4) { - if (this.status === 200) { - if (type === "xml") { - resolve(this.responseXML); - } else { - resolve(JSON.parse(this.response)); - } - } else { - reject(request); - } - } - }; - request.send(); - } - }); - }; - - return getData(this.config.mockData); + const mockData = this.config.mockData; + if (mockData) { + const data = mockData.substring(1, mockData.length - 1); + return JSON.parse(data); + } else { + const response = await fetch(url); + const data = await response.text(); + if (type === "xml") { + return new DOMParser().parseFromString(data, "text/html"); + } else { + return JSON.parse(data); + } + } } });