Skip to content

[Map][Translator][Turbo] Ensure TypeScript code is valid #2562

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

Merged
merged 5 commits into from
Feb 12, 2025

Conversation

Kocal
Copy link
Member

@Kocal Kocal commented Feb 10, 2025

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

This PR aims to fail the build (and the CI) when TypeScript code contain issues.

I also fixed existing issues in Map, Translator, and Turbo code.

Before:

Cleaning up the "/home/runner/work/ux/ux/src/Chartjs/assets/dist" directory...
Building JavaScript files from @symfony/ux-chartjs package...
Done in 3.175 seconds.
Cleaning up the "/home/runner/work/ux/ux/src/Autocomplete/assets/dist" directory...
Building JavaScript files from @symfony/ux-autocomplete package...
Done in 3.179 seconds.
Cleaning up the "/home/runner/work/ux/ux/src/Cropperjs/assets/dist" directory...
Building JavaScript files from @symfony/ux-cropperjs package...
Minifying CSS...
Done in 2.377 seconds.
Cleaning up the "/home/runner/work/ux/ux/src/Dropzone/assets/dist" directory...
Building JavaScript files from @symfony/ux-dropzone package...
Minifying CSS...
Done in 2.434 seconds.
Cleaning up the "/home/runner/work/ux/ux/src/LazyImage/assets/dist" directory...
Building JavaScript files from @symfony/ux-lazy-image package...
Done in 2.438 seconds.
Cleaning up the "/home/runner/work/ux/ux/src/LiveComponent/assets/dist" directory...
Building JavaScript files from @symfony/ux-live-component package...
Minifying CSS...
Done in 3.861 seconds.
Cleaning up the "/home/runner/work/ux/ux/src/Map/assets/dist" directory...
Building JavaScript files from @symfony/ux-map package...
Done in 2.515 seconds.
Cleaning up the "/home/runner/work/ux/ux/src/Map/src/Bridge/Google/assets/dist" directory...
Building JavaScript files from @symfony/ux-google-map package...
Done in 2.509 seconds.
[plugin typescript] src/map_controller.ts (12:35): @rollup/plugin-typescript TS2307: Cannot find module '@symfony/ux-map' or its corresponding type declarations.
[plugin typescript] src/map_controller.ts (19:8): @rollup/plugin-typescript TS2307: Cannot find module '@symfony/ux-map' or its corresponding type declarations.
[plugin typescript] src/map_controller.ts (78:21): @rollup/plugin-typescript TS7053: Element implicitly has an 'any' type because expression of type 'Library' can't be used to index type 'typeof maps'.
  Property 'streetView' does not exist on type 'typeof maps'.
[plugin typescript] src/map_controller.ts (80:55): @rollup/plugin-typescript TS2698: Spread types may only be created from object types.
[plugin typescript] src/map_controller.ts (89:30): @rollup/plugin-typescript TS2339: Property 'hasCenterValue' does not exist on type 'default'.
[plugin typescript] src/map_controller.ts (89:53): @rollup/plugin-typescript TS2339: Property 'centerValue' does not exist on type 'default'.
[plugin typescript] src/map_controller.ts (90:37): @rollup/plugin-typescript TS2339: Property 'centerValue' does not exist on type 'default'.
[plugin typescript] src/map_controller.ts (95:30): @rollup/plugin-typescript TS2339: Property 'hasZoomValue' does not exist on type 'default'.
[plugin typescript] src/map_controller.ts (95:51): @rollup/plugin-typescript TS2339: Property 'zoomValue' does not exist on type 'default'.
[plugin typescript] src/map_controller.ts (96:35): @rollup/plugin-typescript TS2339: Property 'zoomValue' does not exist on type 'default'.
[plugin typescript] src/map_controller.ts (101:14): @rollup/plugin-typescript TS2339: Property 'dispatch' does not exist on type 'default'.
[plugin typescript] src/map_controller.ts (125:42): @rollup/plugin-typescript TS2339: Property 'element' does not exist on type 'default'.
[plugin typescript] src/map_controller.ts (148:18): @rollup/plugin-typescript TS2551: Property 'createInfoWindow' does not exist on type 'default'. Did you mean 'doCreateInfoWindow'?
[plugin typescript] src/map_controller.ts (176:18): @rollup/plugin-typescript TS2551: Property 'createInfoWindow' does not exist on type 'default'. Did you mean 'doCreateInfoWindow'?
[plugin typescript] src/map_controller.ts (204:18): @rollup/plugin-typescript TS2551: Property 'createInfoWindow' does not exist on type 'default'. Did you mean 'doCreateInfoWindow'?
[plugin typescript] src/map_controller.ts (264:18): @rollup/plugin-typescript TS2339: Property 'markers' does not exist on type 'default'.
[plugin typescript] src/map_controller.ts (269:14): @rollup/plugin-typescript TS2339: Property 'markers' does not exist on type 'default'.
[plugin typescript] src/map_controller.ts (269:31): @rollup/plugin-typescript TS7006: Parameter 'marker' implicitly has an 'any' type.
[plugin typescript] src/map_controller.ts (296:14): @rollup/plugin-typescript TS2339: Property 'infoWindows' does not exist on type 'default'.
[plugin typescript] src/map_controller.ts (296:35): @rollup/plugin-typescript TS7006: Parameter 'otherInfoWindow' implicitly has an 'any' type.
Cleaning up the "/home/runner/work/ux/ux/src/Map/src/Bridge/Leaflet/assets/dist" directory...
Building JavaScript files from @symfony/ux-leaflet-map package...
Done in 2.459 seconds.
"leaflet/dist/leaflet.min.css" is imported by "src/map_controller.ts", but could not be resolved – treating it as an external dependency.
Cleaning up the "/home/runner/work/ux/ux/src/Notify/assets/dist" directory...
Building JavaScript files from @symfony/ux-notify package...
Done in 2.381 seconds.
Cleaning up the "/home/runner/work/ux/ux/src/React/assets/dist" directory...
Building JavaScript files from @symfony/ux-react package...
Done in 2.498 seconds.
Cleaning up the "/home/runner/work/ux/ux/src/StimulusBundle/assets/dist" directory...
Building JavaScript files from @symfony/stimulus-bundle package...
Done in 2.444 seconds.
Cleaning up the "/home/runner/work/ux/ux/src/Svelte/assets/dist" directory...
Building JavaScript files from @symfony/ux-svelte package...
Done in 2.453 seconds.
Cleaning up the "/home/runner/work/ux/ux/src/Swup/assets/dist" directory...
Building JavaScript files from @symfony/ux-swup package...
Done in 2.254 seconds.
Cleaning up the "/home/runner/work/ux/ux/src/TogglePassword/assets/dist" directory...
Building JavaScript files from @symfony/ux-toggle-password package...
Minifying CSS...
Done in 2.489 seconds.
Cleaning up the "/home/runner/work/ux/ux/src/Translator/assets/dist" directory...
Building JavaScript files from @symfony/ux-translator package...
Done in 2.351 seconds.
[plugin typescript] src/formatters/intl-formatter.ts (26:5): @rollup/plugin-typescript TS2[32](https://github.com/symfony/ux/actions/runs/13231659169/job/36929714748#step:5:37)2: Type 'string | number | (string | number)[]' is not assignable to type 'string'.
  Type 'number' is not assignable to type 'string'.
[plugin typescript] src/translator.ts (152:57): @rollup/plugin-typescript TS2[34](https://github.com/symfony/ux/actions/runs/13231659169/job/36929714748#step:5:39)5: Argument of type 'ParametersType' is not assignable to parameter of type 'Record<string, string | number>'.
  Type 'Record<string, string | number | Date>' is not assignable to type 'Record<string, string | number>'.
    'string' index signatures are incompatible.
      Type 'string | number | Date' is not assignable to type 'string | number'.
[plugin typescript] src/translator.ts (166:49): @rollup/plugin-typescript TS2345: Argument of type 'ParametersType' is not assignable to parameter of type 'Record<string, string | number>'.
  Type 'Record<string, string | number | Date>' is not assignable to type 'Record<string, string | number>'.
Cleaning up the "/home/runner/work/ux/ux/src/Turbo/assets/dist" directory...
Building JavaScript files from @symfony/ux-turbo package...
Done in 2.2[49](https://github.com/symfony/ux/actions/runs/13231659169/job/36929714748#step:5:54) seconds.
[plugin typescript] src/turbo_stream_controller.ts (11:61): @rollup/plugin-typescript TS7016: Could not find a declaration file for module '@hotwired/turbo'. '/home/runner/work/ux/ux/node_modules/@hotwired/turbo/dist/turbo.es2017-umd.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/hotwired__turbo` if it exists or add a new declaration (.d.ts) file containing `declare module '@hotwired/turbo';`
Cleaning up the "/home/runner/work/ux/ux/src/Typed/assets/dist" directory...
Building JavaScript files from @symfony/ux-typed package...
Done in 2.228 seconds.
Cleaning up the "/home/runner/work/ux/ux/src/Vue/assets/dist" directory...
Building JavaScript files from @symfony/ux-vue package...
Done in 1.970 seconds.
Done in 30s 328ms

After:

Cleaning up the "/home/runner/work/symfony-ux/symfony-ux/src/Chartjs/assets/dist" directory...
Building JavaScript files from @symfony/ux-chartjs package...
Done in 2.830 seconds.
Cleaning up the "/home/runner/work/symfony-ux/symfony-ux/src/Autocomplete/assets/dist" directory...
Building JavaScript files from @symfony/ux-autocomplete package...
Done in 2.967 seconds.
Cleaning up the "/home/runner/work/symfony-ux/symfony-ux/src/Cropperjs/assets/dist" directory...
Building JavaScript files from @symfony/ux-cropperjs package...
Minifying CSS...
Done in 2.[39](https://github.com/Kocal/symfony-ux/actions/runs/13252531380/job/36993303948?pr=2562#step:5:44)2 seconds.
Cleaning up the "/home/runner/work/symfony-ux/symfony-ux/src/Dropzone/assets/dist" directory...
Building JavaScript files from @symfony/ux-dropzone package...
Minifying CSS...
Done in 2.[42](https://github.com/Kocal/symfony-ux/actions/runs/13252531380/job/36993303948?pr=2562#step:5:47)8 seconds.
Cleaning up the "/home/runner/work/symfony-ux/symfony-ux/src/LazyImage/assets/dist" directory...
Building JavaScript files from @symfony/ux-lazy-image package...
Done in 2.472 seconds.
Cleaning up the "/home/runner/work/symfony-ux/symfony-ux/src/LiveComponent/assets/dist" directory...
Building JavaScript files from @symfony/ux-live-component package...
Minifying CSS...
Done in 3.855 seconds.
Cleaning up the "/home/runner/work/symfony-ux/symfony-ux/src/Map/assets/dist" directory...
Building JavaScript files from @symfony/ux-map package...
Done in 2.[47](https://github.com/Kocal/symfony-ux/actions/runs/13252531380/job/36993303948?pr=2562#step:5:52)1 seconds.
Cleaning up the "/home/runner/work/symfony-ux/symfony-ux/src/Notify/assets/dist" directory...
Building JavaScript files from @symfony/ux-notify package...
Done in 2.392 seconds.
Cleaning up the "/home/runner/work/symfony-ux/symfony-ux/src/React/assets/dist" directory...
Building JavaScript files from @symfony/ux-react package...
Done in 2.597 seconds.
Cleaning up the "/home/runner/work/symfony-ux/symfony-ux/src/StimulusBundle/assets/dist" directory...
Building JavaScript files from @symfony/stimulus-bundle package...
Done in 2.[48](https://github.com/Kocal/symfony-ux/actions/runs/13252531380/job/36993303948?pr=2562#step:5:53)4 seconds.
Cleaning up the "/home/runner/work/symfony-ux/symfony-ux/src/Svelte/assets/dist" directory...
Building JavaScript files from @symfony/ux-svelte package...
Done in 2.470 seconds.
Cleaning up the "/home/runner/work/symfony-ux/symfony-ux/src/Swup/assets/dist" directory...
Building JavaScript files from @symfony/ux-swup package...
Done in 2.264 seconds.
Cleaning up the "/home/runner/work/symfony-ux/symfony-ux/src/TogglePassword/assets/dist" directory...
Building JavaScript files from @symfony/ux-toggle-password package...
Minifying CSS...
Done in 2.[57](https://github.com/Kocal/symfony-ux/actions/runs/13252531380/job/36993303948?pr=2562#step:5:62)2 seconds.
Cleaning up the "/home/runner/work/symfony-ux/symfony-ux/src/Translator/assets/dist" directory...
Building JavaScript files from @symfony/ux-translator package...
Done in 2.[58](https://github.com/Kocal/symfony-ux/actions/runs/13252531380/job/36993303948?pr=2562#step:5:63)1 seconds.
Cleaning up the "/home/runner/work/symfony-ux/symfony-ux/src/Turbo/assets/dist" directory...
Building JavaScript files from @symfony/ux-turbo package...
Done in 2.296 seconds.
Cleaning up the "/home/runner/work/symfony-ux/symfony-ux/src/Typed/assets/dist" directory...
Building JavaScript files from @symfony/ux-typed package...
Done in 2.273 seconds.
Cleaning up the "/home/runner/work/symfony-ux/symfony-ux/src/Vue/assets/dist" directory...
Building JavaScript files from @symfony/ux-vue package...
Done in 2.083 seconds.
Cleaning up the "/home/runner/work/symfony-ux/symfony-ux/src/Map/src/Bridge/Google/assets/dist" directory...
Building JavaScript files from @symfony/ux-google-map package...
Done in 2.579 seconds.
Cleaning up the "/home/runner/work/symfony-ux/symfony-ux/src/Map/src/Bridge/Leaflet/assets/dist" directory...
Building JavaScript files from @symfony/ux-leaflet-map package...
Done in 2.742 seconds.
"leaflet/dist/leaflet.min.css" is imported by "src/map_controller.ts", but could not be resolved – treating it as an external dependency.
Done in 30s [68](https://github.com/Kocal/symfony-ux/actions/runs/13252531380/job/36993303948?pr=2562#step:5:73)6ms

@carsonbot carsonbot added Bug Bug Fix Map Turbo Status: Needs Review Needs to be reviewed labels Feb 10, 2025
Copy link

github-actions bot commented Feb 10, 2025

📊 Packages dist files size difference

Thanks for the PR! Here is the difference in size of the packages dist files between the base branch and the PR.
Please review the changes and make sure they are expected.

FileBefore (Size / Gzip)After (Size / Gzip)
Map (Bridge Google)
map_controller.js 10.02 kB / 2.22 kB 10.16 kB+1% 📈 / 2.25 kB+1% 📈
Translator
formatters/formatter.d.ts 113 B / 122 B 120 B+6% 📈 / 129 B+6% 📈
formatters/intl-formatter.d.ts 117 B / 129 B 124 B+6% 📈 / 137 B+6% 📈
utils.d.ts 102 B / 110 B 109 B+7% 📈 / 116 B+5% 📈

@@ -40,6 +40,7 @@
"devDependencies": {
"@googlemaps/js-api-loader": "^1.16.6",
"@hotwired/stimulus": "^3.0.0",
"@symfony/ux-map": "workspace:*",
Copy link
Member Author

@Kocal Kocal Feb 10, 2025

Choose a reason for hiding this comment

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

It has no impact on dist/ files, either for Webpack Encore or AssetMapper. Here we are telling Yarn that package @symfony/ux-map is a "parent" package of @symfony/ux-google-map, in order to fix the topological order (see https://yarnpkg.com/cli/workspaces/foreach)

@Kocal Kocal force-pushed the fix-ts-types-when-building branch 2 times, most recently from c600924 to c903aec Compare February 10, 2025 23:39
@Kocal Kocal requested review from kbond and smnandre February 10, 2025 23:40
@Kocal Kocal changed the title [Map][Translator][Turbo] Ensure TypeScript code is valid (and fix it!) [Map][Translator][Turbo] Ensure TypeScript code is valid Feb 11, 2025
@Kocal Kocal force-pushed the fix-ts-types-when-building branch from c903aec to 641ea02 Compare February 11, 2025 07:45
@Kocal Kocal added the DX label Feb 11, 2025
@Kocal Kocal force-pushed the fix-ts-types-when-building branch from 641ea02 to ddbabd6 Compare February 11, 2025 08:02
@smnandre
Copy link
Member

Reaction in one word: thank-you-Kocal-this-is-very-very-nice-and-yes-i-cheated-a-bit 😎

@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Feb 12, 2025
@Kocal Kocal merged commit 63c8809 into symfony:2.x Feb 12, 2025
63 of 64 checks passed
@Kocal Kocal deleted the fix-ts-types-when-building branch February 12, 2025 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Bug Fix DX Map Status: Reviewed Has been reviewed by a maintainer Translator Turbo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants