Leaflet.draw
Leaflet.draw copied to clipboard
Leaflet
Javascript errors with leaflet 1.1.0
- [x] I'm reporting a bug, not asking for help
- [x] I've looked at the documentation to make sure the behaviour is documented and expected
- [x] I'm sure this is a Leaflet Draw code issue, not an issue with my own code nor with the framework I'm using (Cordova, Ionic, Angular, React…)
- [x] I've searched through the issues to make sure it's not yet reported
How to reproduce
- Leaflet version I'm using: 1.1.0
- Leaflet Draw version I'm using: 0.4.9
What behaviour I'm expecting and which behaviour I'm seeing
I expect the javascript to load without throwing errors but it reports:
TypeError: can't define property "segmentsIntersect": Object is not extensible
Minimal example reproducing the issue
Load https://jsfiddle.net/1fuq748y/1/ while watching the console.
Another warning:
Deprecated include of L.Mixin.Events: this property will be removed in future releases, please inherit from L.Evented instead.
We can reproduce the above reports by @tomhughes and @zakjan in our own integrations.
Same issue here.. "segmentsIntersect" error and mixin deprecation warning
For a workaround on the "segmentsIntersect" bug you can pin the Leaflet version in your own package.json at "1.0.3". This will satisfy the semver requirements of the Leaflet-draw package.json and work. Not sure what changed in the Leaflet package between 1.0.3 and 1.1.0. Might go have a quick look and see, but from a quick glance earlier, it seems the answer was 'a lot'.
https://github.com/Leaflet/Leaflet/issues/5589
Does this appear to be the issue? If so, it looks like it's something that Leaflet-Draw itself would have to change.
This is happening for me using Leaflet 1.0.3 and Leaflet-Draw 0.4.9
So what's the solution here? Just making L.LineUtil into its own class seems like it would be just fine? In fact, I don't see why it's extending L.Util to begin with...
On Wed, Jun 28, 2017 at 3:55 PM, Anthony Frehner [email protected] wrote:
Leaflet/Leaflet#5589 https://github.com/Leaflet/Leaflet/issues/5589
Does this appear to be the issue? If so, it looks like it's something that Leaflet-Draw itself would have to change.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Leaflet/Leaflet.draw/issues/739#issuecomment-311814643, or mute the thread https://github.com/notifications/unsubscribe-auth/AAzNEqyZNZN464EqRStaA-deo0CyxYIsks5sItnJgaJpZM4OGj81 .
The update to Leaflet 1.1.0 also appears to break the test suite in Leaflet.draw because of the ES6+Rollup refactor. Should Leaflet.draw be updated for ES6+Rollup as well? Happy to contribute on that but I would assume that that change is relatively large in scope.
If you're volunteering to port over 5k lines of javascript and then reconfigure the build system - then by all means, please do.
On Thu, Jun 29, 2017 at 6:12 PM, Michael Clawar [email protected] wrote:
The update to Leaflet 1.1.0 also appears to break the test suite in Leaflet.draw because of the ES6+Rollup refactor. Should Leaflet.draw be updated for ES6+Rollup as well? Happy to contribute on that but I would assume that that change is relatively large in scope.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Leaflet/Leaflet.draw/issues/739#issuecomment-312148340, or mute the thread https://github.com/notifications/unsubscribe-auth/AAzNEo-GIrihR9KNFpJXUE_4nQeyhFCJks5sJEuFgaJpZM4OGj81 .
Lots of good information here. It does appear the minor release of Leaflet broke Draw.
Any patches to resolve the utility issue should reference #5589 (thanks @frehner for pointing that out).
ES6 Rollup would be nice, we can set up an ES6 branch to handle that. Leaflet.Editable is having a similar issue (looks like utility) and the next 'major' update to Leaflet.Draw will be piggybacking off Editable - just an FYI.
In the meantime, I'll scope the compatibility of Leaflet.Draw to a ceiling of Leaflet 1.0.x. This will happen tomorrow morning.
@midnightmastermind - was just thinking about why you'd still have the problem. Are you using npm3 or greater? Because npm3 stores all dependencies in a flat structure, so by adding Leaflet to your own package.json, it should not be refetched by Leaflet Draw. npm2 stores each dependencies' dependencies (?) in their own node_modules folder, so would be refetched. Hope that makes sense.
@JonForest thanks for the heads up. I actually found out the issue was my npm install was downloading the newest version due to the have "^1.0.3" as the dependency version. Removing the carot solved the issue.
@ddproxy I don't know if you had a timeline in mind for progressing on ES6+Rollup but I would be able to dig into that transition next week. I am indeed volunteering to do as much as needed to port over 5k lines of js and reconfigure the build system (and certainly willing to see it to the finish line since we use Leaflet.Draw pretty extensively on projects at StratoDem).
Do you want the ES6+Rollup to wait until Leaflet.Editable is updated, or can that progress in parallel?
@mjclawar so, that implies abandoning #651, among others? What advantages are you expecting from the ES6 port?
At any rate, it sounds like, at this point, if @ddproxy wants to take Leaflet Draw into another direction w/ Leaflet Editable, I should just permanently fork Leaflet Draw into another plugin entirely.
On Fri, Jun 30, 2017 at 2:38 PM, Michael Clawar [email protected] wrote:
@ddproxy https://github.com/ddproxy I don't know if you had a timeline in mind for progressing on ES6+Rollup but I would be able to dig into that transition next week. I am indeed volunteering to do as much as needed to port over 5k lines of js and reconfigure the build system (and certainly willing to see it to the finish line since we use Leaflet.Draw pretty extensively on projects at StratoDem).
Do you want the ES6+Rollup to wait until Leaflet.Editable is updated, or can that progress in parallel?
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Leaflet/Leaflet.draw/issues/739#issuecomment-312378639, or mute the thread https://github.com/notifications/unsubscribe-auth/AAzNElRCQMstBzsUbEwnUBxPNassGPzXks5sJWrhgaJpZM4OGj81 .
There are three different directions that can be taken at this point.
- ES6 Rollup
- Bugfixes for L1.1
- Editable integration
@mjclawar We can do the Rollup without Editable, since the Editable changes haven't made public yet (it's very alpha at this point) both can progress at their own rates.
@germanjoey I've been hesitant to approve PRs that make major changes or implement major features to this plugin. By using Editable, we can get access to their plugins that implement some of the features we are looking for. This should allow new features to remain optional, pluggable, and customizable that the current version of Draw struggles to maintain.
@ddproxy
I've gotta say, suddenly voicing this concern now after sitting on #651 for seven months (and despite my repeated attempts to follow up with you) is ridiculous. The concern about major changes also doesn't make sense, since I took great pains w/ L.Draw's #651 and L.Snap's https://github.com/makinacorpus/Leaflet.Snap/pull/40 to not make any major changes. The only modification to the existing logic was to flesh out the parameters for various L.Draw's events, for which I ensured backwards compatibility and also provided documentation. The other changes - restoring L.Snap compatibility, Undo/Redo, and providing L.CRS.Simple compatibility - were all completely optional/pluggable/customizable/whatever you wanna call it.
I don't really know what you're going for w.r.t. L.Editable integration with L.Draw. Feature-wise, as L.Editable seems to have every feature that L.Draw has except for the Toolbar?
And are you trying to say that you're done with this existing Leaflet Draw codebase?
Joe
On Sat, Jul 1, 2017 at 12:27 PM, Jon West [email protected] wrote:
There are three different directions that can be taken at this point.
- ES6 Rollup
- Bugfixes for L1.1
- Editable integration
@mjclawar https://github.com/mjclawar We can do the Rollup without Editable, since the Editable changes haven't made public yet (it's very alpha at this point) both can progress at their own rates.
@germanjoey https://github.com/germanjoey I've been hesitant to approve PRs that make major changes or implement major features to this plugin. By using Editable, we can get access to their plugins that implement some of the features we are looking for. This should allow new features to remain optional, pluggable, and customizable that the current version of Draw struggles to maintain.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Leaflet/Leaflet.draw/issues/739#issuecomment-312451188, or mute the thread https://github.com/notifications/unsubscribe-auth/AAzNErb4V-MSGttoT19nuYm0M9-P8mP_ks5sJp2JgaJpZM4OGj81 .
@germanjoey
No. Not done with the existing codebase.
#651 introduces the undo and state managers that would qualify as a feature.
However, that is not why #651 has not been merged yet. The PR had a lot of merge conflicts and still does. Once these are resolved and I can test that PR, it may be merged.
@midnightmastermind 0.4.10 was released to handle the leaflet version cap.
@germanjoey I'm going to merge #651 into undo-manager this week. I'm okay with one more minor release (0.5.0) to handle pre Leaflet 1.1 to encompass those changes and ES6 can be 0.6.0. Once I see changes on #651 I'll merge and handle the remaining conflicts if any.
@mjclawar Looking at starting the ES6 rollup, due to bower, this will happen on a branch. If you have anything started, point me to it and I'll merge into es6-rollup
@ddproxy That would be great. I just pushed another commit onto #651 with some bugfixes and additional documentation. Please let me know if there's anything I can do to help.
@ddproxy I wasn't sure what direction this plugin was heading in given the above thread, so I ported Leaflet.draw to an ES6+Babel/Webpack project we use internally. We're probably going to split that out into its own public repo later this week, and I can point you at that when we do if that will be helpful. It has class implementations (e.g., class SimpleShape extends Feature), Flow type annotations and a fair number of ES6 syntax changes, so clearly it can't be merged easily into this plugin. Please let me know what would be most helpful for updates here.
@mjclawar That sounds like it's in the right direction. I favor Babel but since Leaflet uses Rollup, I'll cherrypick what is applicable. Thanks!
Here's where we're at right now: https://github.com/StratoDem/SDLeafletDraw.
Obviously still needs a fair amount of work and I'll try to port the tests soon, but code is working for our use cases with an import 'sdleafletdraw;. This adds L.Control.Draw and L.Draw.Event access.
The ES6 code is in src/es6/, then built with Babel into dist.
@mjclawar Thanks for this. Looks like we stripped a lot of documentation annotations. I'm not sure how Rollup or Babel would treat those (probably ignore).
@ddproxy What do you mean by stripped a lot of documentation annotations? Where/when were they stripped?
@mjclawar There are annotations/code comments above each class and method in the original source code that are used to generate documentation of the api - that produces the api documentation.
What is the fix for this bug? I'm working with a VueJS-PWA project where ES6 and Webpack is imported. When i'm trying to import leaflet and leafletDraw, it gives me the error:
Cannot add property segmentsIntersect, object is not extensible
I have imported it like this:
import Leaflet from 'leaflet';
import LeafletDraw from 'leaflet-draw';
The current versions are:
"leaflet": "^1.1.0",
"leaflet-draw": "^0.4.10"
@Fillah Have you tried https://github.com/Leaflet/Leaflet.draw/issues/739#issuecomment-311659407 ?