Skip to content

Conversation

@KristjanESPERANTO
Copy link
Contributor

@KristjanESPERANTO KristjanESPERANTO commented Nov 22, 2025

This is a big change, but sooner or later this step would have been necessary, I think 🙂

It's difficult to review so much changes properly, I'm sorry. But in small, manageable PRs, it would be a very long process.

What's changed?

Project structure:

  • Source code now lives in src/
  • Generated builds in dist/
  • Rollup builds ESM + UMD versions (via npm run build)

Modernizations:

  • Named imports instead of namespace import (import { Control, DomUtil } from 'leaflet')
  • Proper dual package exports in package.json
  • Add ESM demo
  • New landing page with both demo variants

Breaking Change:
The file structure has completely changed - hence this is a major release. But if users install the package via npm, users shouldn't notice thanks to proper exports configuration.

For users

With bundlers (Vite, Webpack, etc.):

import { FullScreen } from 'leaflet.fullscreen';

Works as before, just with real ESM now.

In the browser with script tags:

<script src="dist/Control.FullScreen.umd.js"></script>

Also works as before, just the file is now called .umd.js instead of .js.

Demos

Check out the new demos:

  • index.html - Landing page
  • demo/demo.esm.html - ESM with Import Maps
  • demo/demo.umd.html - Classic script tags

2025-11-25 Edit: rollback from IIFE to UMD after review.

@KristjanESPERANTO KristjanESPERANTO force-pushed the esm branch 2 times, most recently from 2288ad5 to 63e1326 Compare November 22, 2025 00:29
BREAKING CHANGE: Source files moved to src/ directory, built files in dist/

- Restructure project: source in src/, distribution files in dist/
- Add Rollup build system to generate ES module and IIFE distributions
- Use named imports from Leaflet (Control, DomUtil, DomEvent, Map)
- Configure dual package exports in package.json (ESM + IIFE)
- Add Leaflet as peerDependency (^1.7.0 || >=2.0.0-alpha.1)
- Add demo pages for both ESM and IIFE usage patterns with Import Maps
- Create landing page with feature overview and demo links
- Update README with build command documentation
@brunob
Copy link
Owner

brunob commented Nov 24, 2025

Awesome, thx for this hard work !

Has i've already said before, i'm "a bit" away/oldschool on javascript ecosystem so i plainly trust your choices on module format and tools to use :)

IIFE instead of UMD for browsers (smaller, simpler)

I've tried to update myself on the subject and reading some posts like https://dev.to/fkkarakurt/javascript-module-formats-and-tools-3mg9 & https://javascript.works-hub.com/learn/javascript-modules-358ee let me think that's a good choice + the fact i like smaller/simpler options (y)

New landing page with both demo variants

This refresh should be very welcomed by users, again, thx for that.

You haven't mentioned this point, but if i'm not wrong your PR allow compatibility with future leaflet 2.0 ref #208 and it's great.

Any thought on this @BePo65 ?

@KristjanESPERANTO
Copy link
Contributor Author

Thanks! I'm glad you like the changes 😃

Regarding IIFE:
I see a strong trend toward ESM-only packages in the JavaScript ecosystem. That would indeed be the most consistent approach. However, I've kept the IIFE build for backward compatibility. It doesn't add much overhead for us to maintain, so I think it's okay supporting it for a while (as it is the drop-in replacement for the UMD package).

Regarding Leaflet 2.0 compatibility:
Yes, exactly! With these changes we're now compatible with Leaflet 2.x. I just tested both demos with Leaflet 2.0.0-alpha.1 and they work perfectly:

  • The ESM demo uses leaflet-src.js (which provides ESM exports in Leaflet 2.x)
    "leaflet": "https://unpkg.com/[email protected]/dist/leaflet-src.js"
  • The IIFE demo uses leaflet-global.js (which provides the global L object)
    <script src="https://unpkg.com/[email protected]/dist/leaflet-global.js"></script>

Landing page
I'm also glad that you like this - I've introduced similar ones in leaflet-locatecontrol and Leaflet.markercluster.

So this PR addresses #208 and prepares us for the future but is still compatible with Leaflet 1.x 😃

@KristjanESPERANTO
Copy link
Contributor Author

And this PR addresses #152 as well! It adds explicit CSS export in package.json. And documents both import approaches in the README (bundler import vs. manual HTML link). We can close #152 once this is merged 👍

@BePo65
Copy link
Contributor

BePo65 commented Nov 25, 2025

IMHO this is a great PR and it solves the migration to leaflet@^2 too. And I love the demos 😄.

Perhaps I could add a few ideas:

  1. output formats:
    1.1 all what @KristjanESPERANTO says about iife is absolutely correct. But currently leaflet.fullscreen supports AMD, commonJS and the global L by using the UMD pattern. As Rollup supports all of these formats, IMO we should keep all of these output formats. This way applications with older or more exotic frameworks can continue using this plugin. The only thing that had to be changed is the rollup configuration.
    1.2 As an alternative we could completely switch to an esm-only structure, but this would be an unnecessary break in compatibility.
    My recommendation: add umd as an output format (and therefore optionally drop iife)

  2. content for this package on npmjs
    2.1 IMHO only required files should be published to npmjs. When this package is installed from npmjs, there is no need to install e.g. the build and lint environment too. If someone wants to take a look at the source or modify the code, the recommended way is to fork this repository. So dist should contain all what is to be published on npmjs.
    As a consequence the following files had to be added to the dist directory (e.g by using cpy-cli in the npm build script): readme.md, CHANGELOG.md, LICENSE and above all package,json.
    2.2 I am not completely sure about the "entry points" in the package.json: AFAIK only exports and 'browser' are required. 'exports.import' point to the esm variant, while 'exports.require' and 'browser' would point to the umd variant.
    My recommendation: put all files required by apps to dist

  3. the dist directory is / should be for publishing to the package repository (npmjs.org) only. As the content of this directory can be reproduced at any time by running build again, there is no need to add it to the git repo. My recommendation: add dist to the .gitignore file.

What do you think about this?

@brunob
Copy link
Owner

brunob commented Nov 25, 2025

Thx for the feedback @BePo65 :)

1.1 => good point, if it's easy to maintain these patterns we should do it. We can drop them later when they'll be obsolete.

3 => I was going to say that the dist folder is handy for people who want to get the script from a simple wget on the repo, but i see that Leaflet main branch doesn't provide it anymore, so maybe that's the way to do it now...

@KristjanESPERANTO
Copy link
Contributor Author

KristjanESPERANTO commented Nov 25, 2025

Thanks from me too! 👍

  1. ... My recommendation: add umd as an output format (and therefore optionally drop iife)

That's also fine for me. I'll re-add UMD and drop IIFE 🙂

We can drop them later when they'll be obsolete.

Exactly. In the long term, I see all plugins being ESM only.

  1. content for this package on npmjs ...

I agree to your points. I'll take another look at it later and add a commit as a suggestion.

  1. ... add dist to the .gitignore file

I kind of like it when the build files are in the repo. It makes it easier for me to check the diffs when I do a build after a change. But yes, they don't really belong in the repo. I'll add a commit for that.

- UMD supports AMD, CommonJS and global L (browsers)
- Renamed demo.iife.html to demo.umd.html
- Added require export back to package.json
Build artifacts should not be tracked in git.
They are reproducible via 'npm run build'.
- Add 'files' field to publish only dist/
- Add 'browser' field pointing to UMD
- Add 'exports.require' field pointing to UMD
@KristjanESPERANTO
Copy link
Contributor Author

I just added the changes.

To "2. content for this package on npmjs ..."

I've addressed this by adding the files field to package.json:

"files": ["dist/"]

npm automatically includes package.json, README.md, and LICENSE.md, so we don't need to copy them to dist/.

The package now only publishes what's needed:

$ npm pack --dry-run
npm notice 📦  [email protected]
npm notice Tarball Contents
npm notice 1.1kB LICENSE.md
npm notice 5.5kB README.md
npm notice 831B  dist/Control.FullScreen.css
npm notice 7.1kB dist/Control.FullScreen.js
npm notice 7.9kB dist/Control.FullScreen.umd.js
npm notice 947B  dist/icon-fullscreen.svg
npm notice 1.5kB package.json

I also added the browser and exports.require entry point as suggested.

What do you both think? 🙂

@BePo65
Copy link
Contributor

BePo65 commented Nov 26, 2025

using the files field in package.json is a good idea (as in my own projects I always forget to change directories before publishing a package). So I do not have any more comments 😄 - thanks for your patience

@brunob
Copy link
Owner

brunob commented Nov 26, 2025

So I do not have any more comments 😄

@BePo65 (y) can you approve the PR please ?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants