-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
forecast: default to empty array #25177
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `assets/js/components/Forecast/Chart.vue:61` </location>
<code_context>
- grid: { type: Array as PropType<ForecastSlot[]> },
- solar: { type: Object as PropType<SolarDetails> },
- co2: { type: Array as PropType<ForecastSlot[]> },
+ grid: { type: Array as PropType<ForecastSlot[]>, default: [] },
+ solar: { type: Object as PropType<SolarDetails>, default: [] },
+ co2: { type: Array as PropType<ForecastSlot[]>, default: [] },
</code_context>
<issue_to_address>
**suggestion:** Consider using a factory function for default array values in props.
Use a factory function like `default: () => []` to prevent shared state between component instances.
Suggested implementation:
```
grid: { type: Array as PropType<ForecastSlot[]>, default: () => [] },
solar: { type: Object as PropType<SolarDetails>, default: () => ({}) },
co2: { type: Array as PropType<ForecastSlot[]>, default: () => [] },
```
- If the default for `solar` should be `null` instead of an empty object, use `default: () => null` instead.
- Make sure that the default value for `solar` matches the expected shape of `SolarDetails` if you use an object.
</issue_to_address>
### Comment 2
<location> `assets/js/components/Forecast/Chart.vue:62` </location>
<code_context>
- solar: { type: Object as PropType<SolarDetails> },
- co2: { type: Array as PropType<ForecastSlot[]> },
+ grid: { type: Array as PropType<ForecastSlot[]>, default: [] },
+ solar: { type: Object as PropType<SolarDetails>, default: [] },
+ co2: { type: Array as PropType<ForecastSlot[]>, default: [] },
currency: { type: String as PropType<CURRENCY> },
</code_context>
<issue_to_address>
**issue (bug_risk):** Default value for object prop should be a factory function returning an object, not an array.
The `solar` prop's default value should be an object to match its type. Use `default: () => ({})` instead of an empty array to prevent type errors.
</issue_to_address>
### Comment 3
<location> `assets/js/components/Forecast/Chart.vue:63` </location>
<code_context>
- co2: { type: Array as PropType<ForecastSlot[]> },
+ grid: { type: Array as PropType<ForecastSlot[]>, default: [] },
+ solar: { type: Object as PropType<SolarDetails>, default: [] },
+ co2: { type: Array as PropType<ForecastSlot[]>, default: [] },
currency: { type: String as PropType<CURRENCY> },
selected: { type: String as PropType<ForecastType> },
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Use a factory function for array default values in props.
For consistency and to avoid shared state, use `default: () => []` for the `co2` prop, as with the `grid` prop.
```suggestion
co2: { type: Array as PropType<ForecastSlot[]>, default: () => [] },
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@Maschga Eigentlich sollten wir den Fall "es kommen keine Daten mehr" im Datenmodell und bei der Anzeige genau so behandeln wie den Fall dass schon vom Start an keine Daten vorhanden sind (bspw. kein CO2 konfiguriert). Machen wir hier im Publishing was anders? Kurz: ich würd das Problem gerne an der Quelle adressieren. |
|
@naltatis Jetzt sollte im Backend immer |
Wenn die evcc Website für mehrere Tage ohne Neuladen angezeigt wird und in der Zwischenzeit z.B. GrünStromIndex nicht mehr erreichbar ist, kommt es zu dem Fehler, dass das Vorhersagemodel den Chart nicht mehr anzeigt.
Fehlermeldung im Log:
Kamerabilder:
Toggle me!
Ich gehe davon aus, dass diese Zeilen das Problem sind, wenn
nilzurückgegeben wird.evcc/core/site_tariffs.go
Lines 103 to 114 in b928514
evcc/tariff/tariffs.go
Lines 34 to 45 in b928514
Deshalb habe ich
api.Rates{}hinzugefügt.