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

Not adding optional module header results in 'undefined' as a header #1985

Closed
nigel-daniels opened this issue Apr 13, 2020 · 16 comments
Closed

Comments

@nigel-daniels
Copy link

Problem
I just updated to v2.11.0 and modules that previously had no headers (as expected) are now displaying headers with the word 'undefined'.

Expected behavior
The module configuration says the header is an optional property. If this is not added then no header should be displayed.

@rossburton
Copy link

Yeah, seeing this too. Had to set header: "" in the forecast module so it didn't show a undefined - Truro, UK header.

@sejka
Copy link
Contributor

sejka commented Jun 16, 2020

i believe that's connected to #1897
are you using an older browser by any chance?

Yeah, seeing this too. Had to set header: "" in the forecast module so it didn't show a undefined - Truro, UK header.

did the same but the lines of header still persist :/

@nigel-daniels
Copy link
Author

@sejka, in my case I'm using kweb a minimal kiosk browser. This did work as expected with a back level version of MM it's only started showing 'undefined' since the MM upgrade to 2.11.0. Hence raising the issue as it looks like a change in MM has introduced a regression defect.

@sejka
Copy link
Contributor

sejka commented Jun 16, 2020

exactly, in #1897 i mentioned a commit i found to be problematic: d8b7292#diff-77ab163850ef0fa98d6a9ee9fb6bbcaf
It was released in 2.11.0.
Almost certain it's the same issue

@rossburton
Copy link

FWIW I was seeing this with Safari 13 on macOS.

@nigel-daniels
Copy link
Author

Sure, did you want to reopen #1897 or resolve the regression under this defect?

@sejka
Copy link
Contributor

sejka commented Jun 17, 2020

I've submitted PR #1908 but as you can see in the discussion we haven't found clear solution. Maybe worth another look @MichMich?
btw @nigel-daniels are you running MagicMirror serveronly using docker image? Maybe we could produce legacy-browser-friendly docker image.

@sthuber90
Copy link
Contributor

sthuber90 commented Jun 25, 2020

Looking for a solution as well, as I wanted to use MM to reuse my old tablet. Also open to contribute to a legacy-browser-friendly docker image

@sthuber90
Copy link
Contributor

sthuber90 commented Jul 7, 2020

Apparently the new logger.js does not work on old devices with Safari < 11.

(function (root, factory) {
	if (typeof exports === "object") {
		// add timestamps in front of log messages
		require("console-stamp")(console, "yyyy-mm-dd HH:MM:ss.l");

		// Node, CommonJS-like
		module.exports = factory(root.config);
	} else {
		// Browser globals (root is window)
		root.Log = factory(root.config);
	}
})(this, function (config) {
	let logLevel = {
		info: Function.prototype.bind.call(console.info, console),
		log: Function.prototype.bind.call(console.log, console),
		error: Function.prototype.bind.call(console.error, console),
		warn: Function.prototype.bind.call(console.warn, console),
		group: Function.prototype.bind.call(console.group, console),
		groupCollapsed: Function.prototype.bind.call(console.groupCollapsed, console),
		groupEnd: Function.prototype.bind.call(console.groupEnd, console),
		time: Function.prototype.bind.call(console.time, console),
		timeEnd: Function.prototype.bind.call(console.timeEnd, console),
		timeStamp: Function.prototype.bind.call(console.timeStamp, console)
	};

	logLevel.setLogLevel = function (newLevel) {
		if (newLevel) {
			Object.keys(logLevel).forEach(function (key, index) {
				if (!newLevel.includes(key.toLocaleUpperCase())) {
					logLevel[key] = function () {};
				}
			});
		}
	};

	return logLevel;
});

@MichMich
Copy link
Collaborator

MichMich commented Jul 7, 2020

As mentioned earlier, we don't officially support old browsers, but feel free to send a PR if you have a simple solution.

@sthuber90
Copy link
Contributor

I am trying to. The problem I am having is that the approach from @sejka worked with the last release but does not work with the new release because new issues for old browsers were introduced i.e. logger.js changes. I am aware that you don't want to add babel and/or webpack to your project, even though it would simplify keeping old browsers supported accross releases.

@sejka
Copy link
Contributor

sejka commented Jul 7, 2020

we could add babel as a docker image step. I've tried it some time ago but couldn't set up babel so that before mentioned issues would be solved. (i don't think there's a polyfill for HTMLElement API: style or i couldn't find one)

@sthuber90
Copy link
Contributor

I also experimented around with a docker image that does the pre-compiling. Couldn't find a pre-compile solution for the HTML style element, but I believe we could resolve the style topic in here, if we can make logger.js work without too much hassle. For the docker image discussion, I think we could add it to the "official" docker image, see issue bastilimbach/docker-MagicMirror#47

@kjendal
Copy link

kjendal commented Jul 30, 2020

I think it may be a question of identifying the module that is the offender and adding the header: '', line to its config. For me that was MMM-PIR. Interestingly, it appears to only be modules in "top_bar" position.

@rejas
Copy link
Collaborator

rejas commented Aug 1, 2020

I made a new PR for this issue, would really appreciate for some feedback in #2103

@TarikHaci
Copy link

I still have it in the latest karsten13/magicmirror:latest docker environment when I don't specify the header.

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

No branches or pull requests

8 participants