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

CSS causes issue with Turbopack #9261

Open
4 tasks done
jovermier opened this issue Feb 15, 2024 · 6 comments
Open
4 tasks done

CSS causes issue with Turbopack #9261

jovermier opened this issue Feb 15, 2024 · 6 comments
Labels

Comments

@jovermier
Copy link

Checklist

  • I've looked at the documentation to make sure the behavior isn't documented and expected.
  • I'm sure this is an issue with Leaflet, not with my app or other dependencies (Angular, Cordova, React, etc.).
  • I've searched through the current issues to make sure this hasn't been reported yet.
  • I agree to follow the Code of Conduct that this project adheres to.

Steps to reproduce

Use Turbopack to build the project. Observe the build fails to parse the leaflet CSS.

The issue was closed on turbopack's repo siting it is an upstream issue in leaflet. vercel/turbo#2451

Expected behavior

The build should not fail to parse the leaflet CSS.

Current behavior

Turbopack fails to build

error - [parse] [project-with-next]/node_modules/.pnpm/leaflet@1.8.0/node_modules/leaflet/dist/leaflet.css  [project-with-next]/node_modules/.pnpm/leaflet@1.8.0/node_modules/leaflet/dist/leaflet.css:552:32  Parsing css source code failed
     548   	width: 24px;
     549   	margin: 0 auto;
     550   
     551   	-ms-filter: "progid:DXImageTransform.Microsoft.Matrix(M11=0.70710678, M12=0.70710678, M21=-0.70710678, M22=0.70710678)";
     552 > 	filter: progid:DXImageTransform.Microsoft.Matrix(M11=0.70710678, M12=0.70710678, M21=-0.70710678, M22=0.70710678);
     553   	}
     554   
     555   .leaflet-oldie .leaflet-control-zoom,
     556   .leaflet-oldie .leaflet-control-layers,
  
  Expected Declaration value

Minimal example reproducing the issue

No response

Environment

  • Leaflet version: 1.9.4
  • Browser (with version): na
  • OS/Platform (with version): linux
@jovermier jovermier added bug needs triage Triage pending labels Feb 15, 2024
@mourner
Copy link
Member

mourner commented Feb 15, 2024

Looks like this was originally added in 2011 to make popup tip IE6–8 properly displayed (rotated 45 degrees for the "tip" appearance). 5f63b8d Not sure if we want to care about this anymore cc @Falke-Design. Might consider removing the offending line (or both filter lines) for v1.9.5 — popups should remain accessible even in old IE.

@Falke-Design
Copy link
Member

In v2 it would not apply because we already removed that we set the leaflet-oldie class. So yes we should remove the filters with v2 too.

Leaflet/src/map/Map.js

Lines 1106 to 1112 in 83fb5a4

DomUtil.addClass(container, 'leaflet-container' +
(Browser.touch ? ' leaflet-touch' : '') +
(Browser.retina ? ' leaflet-retina' : '') +
(Browser.ielt9 ? ' leaflet-oldie' : '') +
(Browser.safari ? ' leaflet-safari' : '') +
(this._fadeAnimated ? ' leaflet-fade-anim' : ''));

Also it looks like Leaflet doesn't work with IE6-8 anymore.
IE6 until IE8 have the same error:
grafik

With Edge simulation of IE8 I get the following error:

':' expected

grafik

which points to get lastId(). The object looks like the Util.js class

var tt = {
    __proto__: null,
    extend: l,
    create: R,
    bind: a,
    get lastId() {
        return D
    },
    stamp: h,
    throttle: j,
    wrapNum: H,
    falseFn: u,
    formatNum: i,
    trim: W,
    splitWords: F,
    setOptions: c,
    getParamString: U,
    template: q,
    isArray: d,
    indexOf: G,
    emptyImageUrl: K,
    requestFn: $,
    cancelFn: Q,
    requestAnimFrame: x,
    cancelAnimFrame: r
};

export var lastId = 0;

So in my eyes we can drop support for IE8 and lower with 1.9.5 because it already doesn't work anymore.

@Falke-Design Falke-Design added good first issue Good for newcomers and removed needs triage Triage pending labels Feb 15, 2024
@mourner
Copy link
Member

mourner commented Feb 15, 2024

@Falke-Design yeah, no one really cares about those versions, and even Leaflet website states IE9+ support on the website for quite some time now. Let's just remove that stuff for v1.9.5.

@jonkoops
Copy link
Collaborator

+1 let's throw this in the bin.

@mujypatahy8

This comment was marked as spam.

arvl130 added a commit to arvl130/rrg-freight-services-web that referenced this issue Feb 24, 2024
This is a workaround until this Leaflet issue is resolved:
Leaflet/Leaflet#9261
@AshwinNema
Copy link

Hi,
I am new to open source. Can I take up this issue?

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

No branches or pull requests

6 participants