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

Sablono fork & Remove render queue #205

Merged
merged 37 commits into from
Jun 28, 2020
Merged

Sablono fork & Remove render queue #205

merged 37 commits into from
Jun 28, 2020

Conversation

roman01la
Copy link
Collaborator

@roman01la roman01la commented Apr 16, 2020

Resolves #198

  • Fork Sablono
    • Remove unnecessary code
    • Simplify and improve interpreter perf
  • Go away from Rum's render queue and .forceUpdate re-rendering model

[rum "0.12.0-SNAPSHOT"]
rum {:mvn/version "0.12.0-SNAPSHOT"}

TESTING

  • JVM SSR should work
  • Typing into input fields should work w/o glitches
  • Anything related to local and reactive state updates should work as expected

@roman01la
Copy link
Collaborator Author

roman01la commented Apr 18, 2020

If you already tested 0.11.5-SNAPSHOT and it works fine for you, give a try to 0.12.0-SNAPSHOT which includes a major update described above.

cc @piranha @martinklepsch @DjebbZ @sergmarch

@roman01la roman01la changed the title Sablono fork wip Sablono fork & Remove render queue Apr 18, 2020
@zelark
Copy link
Contributor

zelark commented Apr 18, 2020

@roman01la, checked #181, it's gone now.

@roman01la
Copy link
Collaborator Author

@roman01la, checked #181, it's gone now.

Nice! Hopefully relying on React's scheduling mechanism we can finally solve issues related to inputs.

@roman01la
Copy link
Collaborator Author

roman01la commented Apr 18, 2020

Btw, for this and future testing it would be great to compile a list of open source projects that depend on Rum, so we could rum their test suits against Rum (if they have tests of course). Any pointers to such projects?

#208

@ikitommi
Copy link

What happens if user has dependency also to sablono - which code is used? Should the forked version be in a separate ns like rum.sablono.*?

@roman01la
Copy link
Collaborator Author

What happens if user has dependency also to sablono - which code is used? Should the forked version be in a separate ns like rum.sablono.*?

Yes, I'll rename those namespaces later.

@roman01la
Copy link
Collaborator Author

👋 @drikerf Mentioning you since you are the most recent user of Rum according to our readme.

@drikerf
Copy link
Contributor

drikerf commented Apr 22, 2020

@drikerf Mentioning you since you are the most recent user of Rum according to our readme.

Hey! I just tested 0.12.0 in staging and everything seems to work great. I have a marketing page which is rendered on the server and then hydrated and the app which is just client rendered.

Thanks for doing this! And let me know if there's anything specific you want me to test, or other ways I could help out :).

@roman01la
Copy link
Collaborator Author

@drikerf Thanks! Glad to hear that it works for you. There's 3 things I'd like to test, they are listed in the PR's description above.

@drikerf
Copy link
Contributor

drikerf commented Apr 24, 2020

@drikerf Thanks! Glad to hear that it works for you. There's 3 things I'd like to test, they are listed in the PR's description above.

  • JVM SSR
  • Inputs
  • State works as expected. Though I pass one state atom from the root down as a prop and use no local/reactive states.

@roman01la
Copy link
Collaborator Author

@drikerf Thanks a lot!

@sergmarch
Copy link

@roman01la I've checked "0.12.0-SNAPSHOT" and everything works as expected.
Checked:

  • SSR
  • Text input fields
  • Local & reactive states

@DjebbZ
Copy link
Contributor

DjebbZ commented Jun 15, 2020

With all the pandemic/quarantine stuff and its impact on work, I'm a bit late to the party but still pleased to announce that we migrated from 0.11.3 to 0.11.5 successfully, 0.11.5 is now in production, no bug found during our testing and no code needed to be changed !

Next-up hopefully this week I'll play with 0.12.0-SNAPSHOT locally and see how it goes. I'll report here.

@DjebbZ
Copy link
Contributor

DjebbZ commented Jun 15, 2020

By the way, we're using rum with citrus, and excluded rum from citrus (since citrus by default brings its own version of rum). citrus 3.2.3 (latest) works fine with rum 0.11.5. We'll see with rum 0.12.0-SNAPSHOT.

@DjebbZ
Copy link
Contributor

DjebbZ commented Jun 15, 2020

Looks KO at first sight, I have an error, I haven't had the time to deep-dive on it but I had a subscription with a derived-atom that simply crashed client-side (server-side the page worked OK it seems). I will do further investigation, hopefully this week. I don't even know if it's rum alone or the combo rum+citrus.

@roman01la
Copy link
Collaborator Author

Merging this into master and cutting a release
@DjebbZ Please report bugs in a separate issue

@roman01la roman01la merged commit 2da89a6 into gh-pages Jun 28, 2020
@DjebbZ
Copy link
Contributor

DjebbZ commented Jul 12, 2020

Just wanted to mention that I'm not working at the same place anymore and I don't think I'll be using rum. So let me ping @bfontaine member of the team, he'll take care of the rest. Keep up the good work!

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.

Remove Sablono dependency, do templating internally
6 participants