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

Spread initialState in index #111

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

oskarakerlund-consid
Copy link

Så man slipper repetera properties i agnosticRender

Relaterat till #110

@oskarakerlund-consid
Copy link
Author

Vad säger du om denna och #110 @albinohrn ? :)

@albinohrn
Copy link
Contributor

Hej, jag våndas lite över den här frågan och därför har svar uteblivit. Sorry! Jag förstår att det kan vara smidigt för er att ha det, men samtidigt är jag lite osäker på om det är något vi vill predika som default.

Det är viktigt att man förstår vad som händer annars finns det risk att man skickar mer data än nödvändigt till klienten. Därför har vi inte velat ändra till detta som default, bättre att man är tvungen att bestämma vad som skickas med till klienten explicit än att nånting råkar åka med som inte var meningen. Man får ingen varning i TS t ex om man skickar in för mycket i sitt initialState.

const intialState = {
  name,
  message,
  irrelevantProperty: 'value'
};

// Förutsatt att App är den komponenten vi har i boilerplaten
// så kommer inte detta att ge några varningar.
renderToString(<App {...initialState} />);

// Hade jag däremot gjort följande så hade jag fått varningar från TypeScript eftersom att irrelevantProperty inte hanteras av APP
renderToString(<App name={initialState.name} message={initialState.message} irrelevantProperty="value" />);

Däremot om man vet vad man gör, som ni, och tycker att det är smidigt då finns möjligheten att ändra, men vi lutar fortfarande åt att det är bättre att vara explicit som default.

@oskarakerlund-consid
Copy link
Author

Tack för utförligt svar @albinohrn - förstår hur du tänker! Det verkar som att båda alternativen har sina fördelar och nackdelar men då är det nog bäst att välja säkerhet framför smidighet. Det ska dock till en riktig tabbe om man råkar sätta något osäkert/onödigt i initialState som man inte vill leverera till App.

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.

3 participants