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

add wrapper scope van #10

Closed

Conversation

sirenkovladd
Copy link

Add scope for van

Example: vanjs-org/van#333

@Tao-VanJS
Copy link
Member

I was trying to understand what this PR tries to do. If I understand correctly, the purpose of this PR is to avoid passing around the van object while building components shared between SSR and CSR. The PR introduced the notion of van object context so that we can call getVan function to get the van object instead of having to pass it around.

Sample code with the feature in this PR will look like that:

In shared components:

const Component = props => {
  const van = getVan()
  const {div, span} = van.tags
  return div(
    span(...)
    span(...)
  )
}

In the main file:

van.add(document.body, withVan(van, () => div(
  Component(...),
  Component(...),
))

My personal take about this approach:

  1. The support of van object context doesn't need to be implemented in the core library. I think this can be achieved in a standalone VanJS add-on.
  2. The idea of context can be even more generalized. We can leverage the notion of context to pass around any global object, not just the van object. There is a relevant discussion about general-purpose context in the past. I didn't follow through the entire thread, but this could be something interesting to read: VanJS Component API van#173.

@sirenkovladd
Copy link
Author

I thought that one of the goals of this repository is to provide a better experience for SSRs and this WP allows us to improve the experience of building components shared between SSRs and CSRs
that's why I thought I would add it here.

I agree that the proposed context is similar to VanJS add-on but wrapping each function to store the context looks very complicated
The main goal of this PR is to allow components to be shared between SSR and CSR with minimal changes

@Tao-VanJS
Copy link
Member

I agree with you that this feature provides better developer experience for some people and some use cases. Appreciate it. Are you comfortable to implement the feature as a standalone add-on of VanJS? This way it will be easier for the author to manage the documentation and future evolution of the add-on.

@sirenkovladd
Copy link
Author

I'm afraid that hundreds of libraries that will add one feature at a time that helps to better interact with this particular library may discourage future developers

and I doubt that any improvements will be needed, this feature looks complete

the only thing that will be possible is to expand the SSR documentation page

@Tao-VanJS
Copy link
Member

My view is: using context instead of parameter passing is an opinionated approach. Objectively speaking, there are pros and cons for each approach, either context or parameter passing.

The general philosophy is: if some people prefer one programming flavor over the other, they can extend VanJS with add-ons. I understand people's preferences and appreciate their contribution to VanJS, but the core library of VanJS is kept neutral over different programming flavors.

There are already add-ons for that exact purpose: van-jsx (JSX flavor for DOM tree composition), van_dml (DML flavor for DOM tree composition).

@thednp
Copy link
Contributor

thednp commented May 29, 2024

Thanks for tagging me in.

@sirenkovladd perhaps updating the examples for us mortals without bun would do to understand and hopefully make time to test 🫦

I really think both context API and this wrapper scope are essential for application development. Context API is always useful for all scopes from global application scope down to the smallest component, while the wrapper makes sure all components are reliably and easily rendered without too much hustle for your regular Joe. A cool idea would be a combination of both, optimized and properly documented.

Guys, the surprise I had for you was an SSR example I was working on, perhaps for create-vite-extra, but as explained above I find it a bit frustrating NGL.

Just my 2cents.

@Tao-VanJS
Copy link
Member

Hi @sirenkovladd / @thednp,

Mini-Van 0.6.0 was just released, which improves the developer experience of fullstack rendering. Among other things, it allows van object to be registered via registerEnv so that the van object doesn't need to be passed around shared components via parameters. Thus with Mini-Van 0.6.0, we now have a solution for the problem this PR tries to solve.

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