Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[Map] Adjust changelogs and fix render_map deprecated version #2138

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

Kocal
Copy link
Member

@Kocal Kocal commented Sep 9, 2024

Q A
Bug fix? no
New feature? no
Issues Fix #...
License MIT

While working on #2070, I noticed Map's changelogs were still containing Unreleased instead of the version number.
I've also changed the version where render_map twig function have been deprecated, to match the reality (the 2.20 will be the next release!).

Copy link
Member

@smnandre smnandre left a comment

Choose a reason for hiding this comment

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

I think "Unreleased" was just a convention (because some component have no changes for multiple versions) so this was to avoid writing wrong "previous + 1" versions in the changelog.

Now i'm 100% ok with both methods, and we'll have a better look for next release :)

@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Sep 9, 2024
@smnandre
Copy link
Member

smnandre commented Sep 9, 2024

I'm more concerned by the failing js tests :|

@Kocal
Copy link
Member Author

Kocal commented Sep 9, 2024

I'm more concerned by the failing js tests :|

I gonna look asap

@Kocal Kocal merged commit e84a96e into symfony:2.x Sep 9, 2024
40 of 41 checks passed
@Kocal Kocal deleted the map-changelogs branch September 9, 2024 21:50
Kocal added a commit that referenced this pull request Sep 10, 2024
…(Kocal)

This PR was squashed before being merged into the 2.x branch.

Discussion
----------

Upgrade Vitest, use a real browser for Map Bridges tests

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Issues        | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT

<!--
Replace this notice by a description of your feature/bugfix.
This will help reviewers and should be a good start for the documentation.

Additionally (see https://symfony.com/releases):
 - Always add tests and ensure they pass.
 - For new features, provide some code snippets to help understand usage.
 - Features and deprecations must be submitted against branch main.
 - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry
 - Never break backward compatibility (see https://symfony.com/bc).
-->

Follow #2138 (comment).

It looks like Google Maps changed a bit their implementation and now manipulates [`SVGImageElement`](developer.mozilla.org/en-us/docs/web/api/svgimageelement) and [`SVGAnimatedString`](https://developer.mozilla.org/en-US/docs/Web/API/SVGAnimatedString):
<img width="1559" alt="image" src="https://github.com/user-attachments/assets/3d82559d-500a-45fa-9730-1a1ccd3adde8">

But, this API is not supported by https://github.com/capricorn86/happy-dom.

Instead of reimplementing things ourselves, I think it's more clever to use a **real browser** (here Chromium) through [Playwright](playwright.dev/), when using [Vitest's Browser mode](https://vitest.dev/guide/browser/).

This PR also upgrade Vitest from ^0.34 to ^2.0, and Vite from 4.5.0 to 5.4.3, which comes with https://vitejs.dev/guide/troubleshooting.html#vite-cjs-node-api-deprecated (that I fixed in a second commit)

Commits
-------

eaab0f6 chore: rename vitest.config.js to vitest.config.mjs, to get rid of https://vitejs.dev/guide/troubleshooting.html#vite-cjs-node-api-deprecated
cebd8e2 Upgrade Vitest, use a real browser for Map Bridges tests
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Map Status: Reviewed Has been reviewed by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants