QwikDev / qwik

Instant-loading web apps, without effort
https://qwik.dev
MIT License
20.84k stars 1.31k forks source link

Expose useEnvData with no overloads #2138

Closed robisim74 closed 1 year ago

robisim74 commented 2 years ago

Is your feature request related to a problem?

Initially, envData(formerly userContext), only had a couple of properties. Now the properties have grown, and they can really be very useful.

The problem is that currently useEnvDatadoesn't follow the pattern of the other use methods, and only one property can be accessed at a time:

const url = useEnvData('url');
const requestHeaders = useEnvData('requestHeaders');
[...]

Describe the solution you'd like

As a developer, I'd like to have a function even without overloads that returns the envData object, so that destructuring is also possible:

const envData = useEnvData():
const { url, requestHeaders } = useEnvData();

Describe alternatives you've considered

I don't know other alternatives.

Additional context

@manucorporat If you like the proposal I can do a pr. Thanks

manucorporat commented 1 year ago

i am a little worried of developers using this API haha cuz it's very low level and it will not serialize data, ie it's server only... I am not against this change, even thinking if we should rename this API to:

useServerEnvData()

and deprecate the current API.

The value will not be persisted in client side...

manucorporat commented 1 year ago

cc @mhevery @adamdbradley

robisim74 commented 1 year ago

@manucorporat This API is important precisely because it is available only server side.

It provides information such as: current url, headers, cookies, locale, Qwik City env data, and custom data from onGet. This data is strategic in several cases:

The only alternative to this API is to get the data in adaptors (for example express) and then pass it to the root component. But this requires extra work and is different for each adaptor.

I could do a pr that renames it and documents it too.

shairez commented 1 year ago

@manucorporat agree that adding "server" is better than a more generic name

thanks @robisim74 ! I say, unless anyone else thinks of a better name in the next few days - go for what Manu suggested - userServerEnvData and open that PR

manucorporat commented 1 year ago

yes, under my point of view it's still a low level api, of course, very useful, but easily wrappable by higher level APIs, qwikcity developers should never have to use this API ever in their app, library authors sure!

robisim74 commented 1 year ago

@manucorporat is there another way to access the current url, headers, cookies in root component or in external libraries that don't depend on Qwik City?