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

Replace native structuredClone with custom cloneDeep function #256

Merged
merged 2 commits into from
May 16, 2023

Conversation

Raruto
Copy link
Owner

@Raruto Raruto commented May 14, 2023

Fixes a regression introduced by: #240

Closes: #255

Fixes a regression introduced by: #240

Closes: #255
@Raruto
Copy link
Owner Author

Raruto commented May 14, 2023

@hupe13 let me know.

@Raruto Raruto changed the title Replace structuredClone with a custom cloneDeep function Replace native structuredClone with custom cloneDeep function May 14, 2023
@hupe13
Copy link
Contributor

hupe13 commented May 14, 2023

The error is gone, but I don't see both on my test page and on your example the marker grafik.

@Raruto
Copy link
Owner Author

Raruto commented May 14, 2023

Check this out now and also check out the other examples with this change because it's quite likely that something else could be broken..

@hupe13
Copy link
Contributor

hupe13 commented May 15, 2023

I went through my test pages and didn't find any errors because of this, but I found some errors in my application.
Your example works, but there is a warning that leaflet-rotate is missing.
What should I look for that might be broken?

@Raruto
Copy link
Owner Author

Raruto commented May 16, 2023

there is a warning that leaflet-rotate is missing.

let preferCanvas = map.options.preferCanvas;
let showAll = Math.min(map.getMaxZoom(), options.showAll);
// You should use "leaflet-rotate" to show rotated arrow markers (preferCanvas: false)
if (!preferCanvas && !map.options.rotate) {
console.warn('Missing dependency: "leaflet-rotate"');
}

What should I look for that might be broken?

Vanilla JS and Leaflet JS are not to able clone (or merge) complex data structures (eg. nested objects and instances of classes), that's why we needed to add a deep clone algorithm for the default options (eg: to fix #238 (comment)).

// GOAL: merge "foo.bar.a" and "foo.bar.b" attributes --> { foo: { bar: { a: 'some value', b: 'another value'} } }

const foo_bar_a = { foo: { bar: { a: 'some value'} } };
const foo_bar_b = { foo: { bar: { b: 'another value'} } };

const leaflet_result = L.extend({}, foo_bar_a, foo_bar_b);
const native_result  = Object.assign({}, foo_bar_a, foo_bar_b);

console.log(leaflet_result.foo.bar); // LOGS: { b: 'another value'}
console.log(native_result.foo.bar); // LOGS: { b: 'another value'}

So, taking a quick look, these are some options that I think might break:

markerIcon: L.divIcon({
className: 'elevation-position-marker',
html: '<i class="elevation-position-icon"></i>',
iconSize: [32, 32],
iconAnchor: [16, 16],
}),

wptIcons: {
'': L.divIcon({
className: 'elevation-waypoint-marker',
html: '<i class="elevation-waypoint-icon default"></i>',
iconSize: [30, 30],
iconAnchor: [8, 30],
}),
},

margins: { top: 30, right: 30, bottom: 30, left: 40 },

handlers: [ // <-- A list of: Dynamic imports || "ClassName" || function Name() { return { /* a custom object definition */ } }
'Distance', // <-- same as: import("../src/handlers/distance.js")
'Time', // <-- same as: import("../src/handlers/time.js")
'Altitude', // <-- same as: import("../src/handlers/altitude.js")
'Slope', // <-- same as: import("../src/handlers/slope.js")
'Speed', // <-- same as: import("../src/handlers/speed.js")
'Acceleration', // <-- same as: import("../src/handlers/acceleration.js")
// 'Pace', // <-- same as: import("../src/handlers/pace.js")
// "Heart", // <-- same as: import("../src/handlers/heart.js")
// "Cadence", // <-- same as: import("../src/handlers/cadence.js")
// import('../src/handlers/heart.js'),
import('../src/handlers/cadence.js'),
// import('../src/handlers/pace.js'),
L.Control.Elevation.MyHeart, // <-- see custom functions declared above
// L.Control.Elevation.MyCadence, // <-- see custom functions declared above
L.Control.Elevation.MyPace, // <-- see custom functions declared above
],

I went through my test pages and didn't find any errors because of this, but I found some errors in my application.

I don't think I understand what you mean..

@hupe13
Copy link
Contributor

hupe13 commented May 16, 2023

I went through my test pages and didn't find any errors because of this, but I found some errors in my application.

I don't think I understand what you mean..

I found some errors in my code. (My english is not good, I'm using translators.)

@Raruto
Copy link
Owner Author

Raruto commented May 16, 2023

Ok, so I merge this pull request and then I release a new version so you can easily test all the latest changes together.

Ref: #252, #253, #254, #256 and #228

@Raruto Raruto merged commit b8f9733 into master May 16, 2023
@Raruto Raruto deleted the clone-deep branch May 16, 2023 15:36
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.

Setting marker: 'position-marker' throws Uncaught TypeError: t.icon.createIcon is not a function.
2 participants