ReferenceError: screenfull is not defined

[mrjones03650]

In a react typescript project, after

npm i leaflet.fullscreen npm i screenfull npm i -D @types/leaflet.fullscreen

when I add import 'leaflet.fullscreen';

the project fails in runtime with:

Unhandled Runtime Error ReferenceError: screenfull is not defined

I'm using "leaflet": "^1.7.1", "react-leaflet": "^3.0.5", "leaflet.fullscreen": "^2.0.0", "screenfull": "^5.1.0",

[brunob]

Please read Tips for typescript environments in https://github.com/brunob/leaflet.fullscreen/blob/master/README.md

[mrjones03650]

Thanks for reading my issue. There isn't anything on the tips for typescript that helps me. In fact, I'm doing it exactly as the README suggests, yet I get a runtime javascript error.

[brunob]

Ha, @BePo65 are you sure that your tip is functional ?

[BePo65]

@mrjones03650 can you give some more information? E.g. how does your code look like and what is the exact wording of the error?

[YoshiYo]

I have exactly the same problem after upgrading your library from 1.6.0 to 2.0.0.

Using :

{
  "leaflet": "^1.7.1" // I tried the 1.5.1 too, same problem
  "leaflet.fullscreen": "^2.0.0"
}

Stack trace :

ReferenceError: screenfull is not defined
    at NewClass._createButton (Control.FullScreen.js:80)
    at NewClass.onAdd (Control.FullScreen.js:40)
    at NewClass.addTo (leaflet-src.js:4757)
    at NewClass.addControl (leaflet-src.js:4824)
    at NewClass.<anonymous> (Control.FullScreen.js:141)
    at NewClass.proto.callInitHooks (leaflet-src.js:352)
    at NewClass.initialize (leaflet-src.js:3119)
    at new NewClass (leaflet-src.js:296)
    at Object.createMap [as map] (leaflet-src.js:4691)
    at VueComponent.mounted (kpimap.vue:227)

And for example, my line 227 is :

this.map = L.map(this.id, {
    fullscreenControl: true,
    fullscreenControlOptions: {
        position: 'topleft',
    },
}).setView([0, 0], 2)

I hope it helps 👋🏻

[BePo65]

ok, let me scratch my head ;-) I'm working with Angular and will try to update my test project on stackblitz.

As I am quite busy at the moment, i probably cannot come back with an answer before next week - so please be patient (I hope this issue is not a showstopper for your projects; otherwise give me a hint so that I can try to switch priorities).

[brunob]

Sorry to not be helpful on that type of question, but i'm far away all this stuff. Anyway, you can tick your project on latest v1 of the script while waiting for a fix :)

[YoshiYo]

No problem, @brunob @BePo65 👋🏻

Tell me if you need anything to reproduce the bug.

[BePo65]

I just tried to reproduce my tests that led to the "section" about typescript in the readme. Somehow I mixed up my code and so I had a modified version of fullscreen in my sample project.

This means: the "section" about typescript in the readme must be removed again :-(

For my own project a made a clone of brunob wonderful project for angular that solves the issues (named @bepo65/leaflet.fullscreen); I suppose it is usable for other typescript frameworks too. I will try to keep this clone in sync with brunobs project (if he agrees :-)

[brunob]

This means: the "section" about typescript in the readme must be removed again :-(

So we just need to replace it with a link to your repo (?)

For my own project a made a clone of brunob wonderful project for angular that solves the issues (named @bepo65/leaflet.fullscreen); I suppose it is usable for other typescript frameworks too. I will try to keep this clone in sync with brunobs project (if he agrees :-)

Sorry to put this charge on your side, but i think it's the right option, or maybe i should simply revert the work i've done on v2 and stick on the old legacy code i was using in v1.Feel free to provide yours thoughts on this point.

[BePo65]

So we just need to replace it with a link to your repo (?)

That would be a possible solution (:-) - @bepo65/leaflet.fullscreen

[BePo65]

or maybe i should simply revert the work i've done on v2 and stick on the old legacy code i was using in v1

let me think during the weekend - perhaps it is possible to find a solution for both

[BePo65]

I think I found the reason for this problems:

1. in the factory function of leaflet.fullscreen the package screenfull is accessed via the global window object (e.g. in line 80 - as you know screenfull is a short form of window.screenfull), but screenfull is added to the global window by the IIFE only if we live in a plain browser environment. As a solution we must inject not only leaflet, but also screenfull into the factory function.

2. the source of screenfull only creates a commonjs module or appends screenfull to the window object. But angular (and perhaps even other frameworks that use typescript plus packagers) use amd modules (if available) and in that case the IIFE of leaflet.fullscreen requires both dependencies (leaflet and screenfull) to be amd modules; as screenfull doesn't deliver an amd module, the reference to the package screenfull cannot be resolved. And that is what produces the error.

I solved both points in the pr #82 (learning a lot about amd modules, typescript and angular :-).

[lobaak]

A workaround until #82 is through the door is to add screenfull to window.

I'm using react-leaflet so this might not work for everyone.

import screenfull from 'screenfull';
window.screenfull = screenfull;

[brunob]

Thx for the PR @BePo65 but i don't get why the PR needs so much changes (will comment about this directly on #82).

Anyway, do you think the patch you porposed in https://github.com/brunob/leaflet.fullscreen/issues/77#issuecomment-771717012 is still valid ? It seems a simpler fix instead of the "huge" diff of #82 :)

[BePo65]

Programming this pr I learned a lot about how Angular (as an example of a typescript based framework) resolves dependencies. During my first 'experiments' with issue #77, I made so many modifications that maybe I finally mixed up my environment. So by now I'm quite sure that #77 will not solve the problem.

@brunob - I'm sorry that this looks like a huge diff, but it all boils down to moving screenfull in front of leaflet.fullscreen in Control.FullScreen.js and git is not so good in showing this kind of modification. This modification will make sure that leaflet.fullscreen finds the dependency screenfull in an amd environment (used in typescript environments by webpack).

Additionally screenfull didn't handle amd modules; this required 2 small modifications in the initialization (new lines 7..9) and in the generation of the return value of screenfull (new lines 158..180).

Perhaps it is easier to look at the list of commits - I didn't squash them and tried to write quite detailed commit messages. The described modification happens in commit bbe9bda.

The modifications to leaflet.fullscreen are:

• inject 'leaflet' and 'screenfull' in the IIFE initialization (as getting these dependencies depends on the used module system) (new lines 189-197)
• return an object with leaflet and screenfull (as webpak - used in e.g. many typescript environments - cannot handle named amd modules) (see commit 6d4f7e9)..

Hope this helps to explain what I did. I think this will solve the 'typescript issues' and still keep your initial objectives (only 1 file; no external dependency, but use screenfull as a base). The only point is that you have to scroll further down the file Control.FullScreen.js to find your code :-)

*** If You like, I can try to split the commit bbe9bda, so that the single steps can be easier identified. Steps are: move screenfull to the start of the file, make screenfull also act as an amd module, inject al dependencies into leaflet.fullscreen, make leaflet.fullscreen return an object that also includes screenfull.

[BePo65]

Just as an additional comment:

1. I think your code got much cleaner by using screenfull instead of the original implementation of the fullscreen interface.

2. IMHO it is 'best practice' to follow the recommendations from the Leaflet Plugin Authoring Guide concerning Module Loaders, even if it is hard sometimes, to build for frameworks that I do not use on a daily basis - but that is what open source is for :-)

[brunob]

*** If You like, I can try to split the commit bbe9bda, so that the single steps can be easier identified. Steps are: move screenfull to the start of the file, make screenfull also act as an amd module, inject al dependencies into leaflet.fullscreen, make leaflet.fullscreen return an object that also includes screenfull.

@BePo65 Thx for your time explaining this, i really appreciate :)

I'll check the diff locally and think we could cherry pick https://github.com/brunob/leaflet.fullscreen/pull/82/commits/bbe9bdadae77243a13c10f4a8ec11d32530d18a5 & https://github.com/brunob/leaflet.fullscreen/pull/82/commits/6d4f7e9fc517e78a294ec2a86bb1e5b59595c66e

By the way, i found this https://github.com/sindresorhus/screenfull.js/pull/35 that could have saved us from all this.

[BePo65]

By the way, i found this sindresorhus/screenfull.js#35 that could have saved us from all this.

But somehow I cannot find the code for exporting 'screenfull' as commonjs module ('require') - but what the heck :-)

[BePo65]

I'll check the diff locally

Thinking about your comments I agree that the commit bbe9bda is too large. So I split it into smaller ones and hope it makes the changes easier to analyze. Pushed the modified version - unfortunately this means that the commit numbers change. But I think it is worth the hassle.

[brunob]

@BePo65 no problem, i'll check the PR asap.

[skirridsystems]

Any further progress on this one? I was seeing something which looked very similar to this when my WordPress plugin is used on iOS/Safari.

[Error] TypeError: undefined is not an object (evaluating 'screenfull.raw.fullscreenchange')
    _createButton (Control.FullScreen.min.js:1:1934)
    onAdd (Control.FullScreen.min.js:1:653)
    addTo (leaflet.js:5:46460)
    addControl (leaflet.js:5:46969)
    (anonymous function) (Control.FullScreen.min.js:1:3507)
    (anonymous function) (leaflet.js:5:3515)
    initialize (leaflet.js:5:26308)
    i (leaflet.js:5:2622)
    (anonymous function) (leaflet.js:5:141670)
    init_datahub_map0 (wordpress-plugin-for-ordnance-survey-maps:64)
    Global Code (wordpress-plugin-for-ordnance-survey-maps:108:105)

[BePo65]

The Problem is solved. Unluckily the (not so substantial) changes contain the reordering of functions in a module and this is not so easy to understand if you have to do this in github.

So please give brunob some more time to do this checking.

[elangobharathi]

I have created a plugin for react-leaflet v3 to integrate with this awesome repo. Please check this out - https://github.com/elangobharathi/react-leaflet-fullscreen-plugin

[ursforrer]

When is the timeline to merge the pull request and publish a new version to npmjs?

[brunob]

When is the timeline to merge the pull request and publish a new version to npmjs?

As soon as i'll find a moment to work on this for people using things that i don't use :)

[brunob]

@BePo65 sorry for the delay, i've found a bit of time too look at this, rebased a bit your work and put it in #94

Does it look good from your side ? Hope we can merge this and release asap :)

[BePo65]

Looks good to me, but give me a day or 3 so that I can run a short test in my test app.

[BePo65]

@brunob my 'test project' runs fine; so feel free to merge & release. Thank you for accepting my solution.

[brunob]

\o/ thx for the feedback @BePo65 :)

Thank you for accepting my solution.

No, i insist, i thank you for your contribution and your patience on this case.

I would also appreciate that every people who have commenting this issue take time to test the PR and confirm that it fix the bug in their environment before releasing, but i maybe i ask too much... Anyway, i think i'll release it on monday.

[brunob]

One last question @BePo65 : do you think we still need all the "Tips for typescript environments:" in the readme after that ? I would too keep this file concise.

[BePo65]

Good question :-) It nearly doubles the length of the README.md without given any 'leaflet.fullscreen' specific information.

So perhaps it would be easier to remove this section from the README and wait for issues, if typescript user should really have problems with this important plugin.

[brunob]

FTR @BePo65 i've just published all this to v2.1.0, thx again for your time and help on this !