Skip to content

Conversation

@umgfoin
Copy link
Contributor

@umgfoin umgfoin commented Jul 14, 2025

Prepare release 1.6.0

@umgfoin umgfoin force-pushed the v1.6.0-Nextcloud-31 branch from 6f6dc8b to 9268101 Compare July 14, 2025 12:38
@umgfoin umgfoin requested a review from tacruc July 14, 2025 12:39
@umgfoin umgfoin force-pushed the v1.6.0-Nextcloud-31 branch from 9268101 to 620e3ac Compare July 14, 2025 12:51
@umgfoin
Copy link
Contributor Author

umgfoin commented Jul 14, 2025

@tacruc, FYI: 'was following your conversations with Mr. Copilot with great interest. A fix for the problematic assignment-operator in FavoritesController is part of this PR.

Can you give me a hint how to fix CI

  • PHP needs to be a minimum version of PHP 7.4.0 and maximum version of PHP 8.3.*.
  • Error: Cannot find module 'node:child_process'
  • npm ERR! Cannot read property '@fortawesome/fontawesome-free' of undefined
    Solved: Fixed with npm i --lockfile-version 2 --package-lock-only

I'm able to build the project locally without dep. child_process.

Concerning PR #1223 / your branch (HEIC-support):
Upstream discussions seem to vote for underlying support in PHP 8.5, so this should probably be still postponed.

@tacruc
Copy link
Collaborator

tacruc commented Jul 14, 2025

Hi @umgfoin,
Thank you very much for your contribution. I will give my best to give you some more detailed Feedback and help over the weekend.

did you check if the map was loading and working after:

  • Update @raruto/leaflet-elevation: 1.9.6 -> 2.5.1
    In my previous attempts this update broke the UI and I didn't manage in reasonable time to fix it.

Are you reachable on talk.nextcloud.com or some other IM?

@umgfoin umgfoin force-pushed the v1.6.0-Nextcloud-31 branch 3 times, most recently from 574719f to 976b19c Compare July 15, 2025 11:21
@umgfoin
Copy link
Contributor Author

umgfoin commented Jul 15, 2025

Hi,

did you check if the map was loading and working after:

* Update @raruto/leaflet-elevation: 1.9.6 -> 2.5.1
  In my previous attempts this update broke the UI and I didn't manage in reasonable time to fix it.

Yes, seems to load and work correctly (subjectively with slightly slower tile loading than 1.9.6) except of the watercolor map.
During testing, I had comparable issues, too - they were amongst others related to broken dependency maps/ package-lock-json etc.). A solution was, to completely remove and re-install (npm) entire packages (not limited to @raruto/leaflet-elevation).
Webpack is complaining (warn) about dynamic d3 imports. I had to remove the static d3-import in Maps.vue.
Not sure, if this is a viable solution?

Are you reachable on talk.nextcloud.com or some other IM?

I'll send you my contact to your github-mail.
++umgfoin.

@umgfoin umgfoin force-pushed the v1.6.0-Nextcloud-31 branch 2 times, most recently from fdac1c2 to c598813 Compare July 18, 2025 14:02
@umgfoin
Copy link
Contributor Author

umgfoin commented Jul 18, 2025

@raruto/leaflet-elevation: 1.9.6 -> 2.5.1
In order to make the elevation-control work correctly with newly introduced "dynamic" imports (since 2.0.0), I had to find a solution to lineup webpack-"linking" and runtime-linking. It ended up in overloading the control's import-methods at runtime aka "monkey-patching". I'm not really happy about this approach.
However, this might be a solution to get rid of other dynamically loaded imports which are imported even in 1.9.6 version from external (unpkg.com). It seems as Julien Veyssier unsuccessfully tried to avoid this originally, when introducing leaflet-elevation.

@umgfoin umgfoin force-pushed the v1.6.0-Nextcloud-31 branch 5 times, most recently from 5983bb2 to aba8120 Compare July 18, 2025 21:25
@umgfoin
Copy link
Contributor Author

umgfoin commented Jul 18, 2025

npm ci still faiiing with:

> @nextcloud/webpack-vue-config@6.3.0 postinstall /home/runner/work/maps/maps/node_modules/@nextcloud/webpack-vue-config
> node ./scripts/postinstall.js

internal/modules/cjs/loader.js:818
  throw err;
  ^

Error: Cannot find module 'node:child_process'

module child_process is installed in dev-section

@flabbamann
Copy link

Hi @umgfoin,
I'm not sure if this relates to your problem, as I don't know much about node 😉, but package.json specifies minimum node version 20, and CI runs with node 12.

node-version: 12.x

umgfoin added 11 commits July 24, 2025 13:22
@nextcloud/auth:                           2.40   -> 2.5.1
@nextcloud/dialogs:                        5.3.1  -> 6.3.1
@nextcloud/moment:                         1.3.2  -> 1.3.4
@nextcloud/sharing:                        0.2.4  -> 0.2.5
@nextcloud/vue:                            8.26.0 -> 8.27.0
@maplibre/maplibre-gl-leaflet:             0.0.22 -> 0.1.1
@raruto/leaflet-elevation:                 1.7.0  -> 1.9.6
axios:                                     1.8.4  -> 1.10.0
leaflet.locatecontrol:                     0.83.0 -> 0.84.2
mapbox-gl:                                 3.4.0  -> 3.13.0
maplibre-gl:                               3.6.2  -> 5.6.1
opening_hours:                             3.8.0  -> 3.9.0
ua-parser-js:                              2.0.0  -> 2.0.4
vue-loader:                                15.11.1 (new)
vue-material-design-icons:                 5.3.0  -> 5.3.1,

dev:
@nextcloud/babel-config:                   1.1.1  -> 1.2.0
@babel/plugin-proposal-object-rest-spread: 7.20.7 (new)
@nextcloud/stylelint-config:               3.0.1  -> 3.1.0
@nextcloud/webpack-vue-config:             6.0.1  -> 6.3.0
@types/leaflet:                            1.9.15 -> 1.9.20

Signed-off-by: umgfoin <umgfoin@users.noreply.github.com>
Several dynamic imports (as of @raruto/leaflet-elevation >=2.0.0) conflict with webpack:
map.vue: Monkey-patch leaflet-elevation's import-method to exclude dynamic-imports from webpack-handling:
Dynamically loaded imports are either served from external-repos (e.g. d3) or locally served at
<maps-root>/src/components/leaflet-elevation/src.
Keep in mind to update components there with every future update of @raruto/leaflet-elevation
and update the monkey-patching according to the original implementation:
@<maps_src_root>node_modules/@raruto/leaflet-elevation/dist/leaflet-elevation.js

Signed-off-by: umgfoin <umgfoin@users.noreply.github.com>
…essbook entries.

Signed-off-by: umgfoin <umgfoin@users.noreply.github.com>
Extend list of evaluated address-categories to 'building', 'place', 'amenity':
In the long run, a migration to geocodejson-format should be evaluated, as the currently returned tags for categories
are given according to OSM tagging which is not very consistent.

Quoting from https://nominatim.org/release-docs/latest/api/Search/:
"If you are interested in a stable classification of address categories (suburb, city, state, etc),
 have a look at the geocodejson format.
All other formats return classifications according to OSM tagging.
There is a much larger set of categories and they are not always consistent, which makes them very hard to work with."

Signed-off-by: umgfoin <umgfoin@users.noreply.github.com>
Signed-off-by: umgfoin <umgfoin@users.noreply.github.com>
Signed-off-by: umgfoin <umgfoin@users.noreply.github.com>
Signed-off-by: umgfoin <umgfoin@users.noreply.github.com>
Signed-off-by: umgfoin <umgfoin@users.noreply.github.com>
Todo: Implement API-key-settings and -authorization.

Signed-off-by: umgfoin <umgfoin@users.noreply.github.com>
Signed-off-by: umgfoin <umgfoin@users.noreply.github.com>
@umgfoin umgfoin force-pushed the v1.6.0-Nextcloud-31 branch from aba8120 to eff3a1e Compare July 24, 2025 11:24
Signed-off-by: umgfoin <umgfoin@users.noreply.github.com>
@umgfoin umgfoin force-pushed the v1.6.0-Nextcloud-31 branch from eff3a1e to 57b2666 Compare July 24, 2025 12:38
@umgfoin
Copy link
Contributor Author

umgfoin commented Jul 24, 2025

but package.json specifies minimum node version 20, and CI runs with node 12.

Many thanks, @flabbamann: This was the culprit!
In particular:
Updated dep @nextcloud/webpack-vue-config: 6.3.0' uses a postinstall script refering to node:child_process in a require-call. The namespace-notation node:xxx is supported not earlier than node >=16.7 ...

"ALL checks have passed"

Copy link
Collaborator

@tacruc tacruc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks alot

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.

4 participants