pnp / sp-dev-fx-controls-react

Reusable React controls for SPFx solutions
https://pnp.github.io/sp-dev-fx-controls-react/
MIT License
382 stars 379 forks source link

Drop dependency to pageContext where possible? #667

Open patrikdevcore opened 3 years ago

patrikdevcore commented 3 years ago

Category

[x] Enhancement

[ ] Bug

[ ] Question

Version

Please specify what version of the library you are using: [any]

Expected / Desired Behavior / Question

A lot/most components require an spfx page context as a prop input. The library would be a lot more useful if you could optionally supply a sp web url instead as this doesn't put a dependency on an spfx solution. In some cases of course this is needed as the component may be utilizing some other components of the spfx project, but a lot of times it seems unncessarry.

The pnpjs sp library does not have this dependency. Neither do the fluent UI controls obviously as they're general fluent UI components. This makes those both librarys highly usable. Most applications I've done in modern sharepoint, I've used a spfx wrapper project for installation and a create-react-app project for business logic as the spfx projects are a pain to develop in. It's just too bad you can't utilize most controls here because of this dependency to the page context.

ghost commented 3 years ago

Thank you for reporting this issue. We will be triaging your incoming issue as soon as possible.

nbelyh commented 3 years ago

@patrikdevcore I have solved this thing by creating a global react context and putting the spfx context there. So you don't need to pass it down to every component that requires it. Instead, you could use it like this in the component (assuming that AppContext is the context class, and you put a field "spfxContext" there at the top comonent render / initialization:

const { spfxContext } = React.useContext(AppContext);

Check out the React docs with example of "theme context". You could just add SPFx context to that sample, should go: https://reactjs.org/docs/hooks-reference.html#usecontext

patrikdevcore commented 3 years ago

@nbelyh thanks for the reply. the problem though is not that i need to pass pageContext around, the problem is that i don't have a pageContext to start with in 90% of my sharepoint projects as the code is not in teh scope of the spfx project which is only used as a wrapper project. instead i normally have a create-react-app project or similar and utilizing pnpjs/sp and fluent ui components.. this all works great and is a lot easier/quicker/better to develop in than the massive overhead and headache of spfx templated projects.

but in order to use the people picker component for instance, i ended up copying all the code replacing pageContext prop to webUrl prop and removing all unnecessarry dependencies. works just as well so i just think it would be nice to remove said dependencies when possible to make the library useful for more scenarios 👍

nbelyh commented 3 years ago

@patrikdevcore I see now 😃 Yes you are right, if you use the controls outside the SPFx app then context could be a nightmare.

patrikdevcore commented 3 years ago

@nbelyh by now i've basically created my own reusable components for most stuff. but it feels kind of silly when i know this library exists and does the same thing. usually the less dependencies the better so i just thought i'd highlight the frustration i've had for the past 2 years.

wilecoyotegenius commented 3 years ago

I fully agree with this proposal. In most of the cases, passing of a full context isn't required to have control fully functional. And as @waldekmastykarz wrote long time ago - it's a bad practice: https://blog.mastykarz.nl/dont-pass-web-part-context-react-components/