-
Notifications
You must be signed in to change notification settings - Fork 984
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
DX: Preload user namespace #19927
DX: Preload user namespace #19927
Conversation
Jenkins BuildsClick to see older builds (20)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good ✅
Thanks for setting this up 🙏
5cb40df
to
31c986f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ilmotta Thank you for this. So many times I reset the user ns accidentally 😞
BTW Would it be possible to have both user and dev.user?
a3d49c8
to
6d17457
Compare
Yes, so many times! I can accommodate your suggestion @clauxx. @seanstrom does that sound okay for you? |
I'm happy to have both too 🙌 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @ilmotta for this PR! 😄
5ec8e17
to
ff5db2a
Compare
85% of end-end tests have passed
Failed tests (6)Click to expandClass TestWalletMultipleDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestWalletOneDevice:
Class TestDeepLinksOneDevice:
Expected to fail tests (2)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (44)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestWalletOneDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
ff5db2a
to
c9544a9
Compare
Reverts the preloading mechanism from PR #19927, which introduced a problem I couldn't find any satisfactory solution. The namespaces user and dev.user were being preloaded by shadow-cljs, but the problem is that preloading will fail if those namespaces don't exist in your file system. I couldn't find any solution to conditionally preload namespaces via shadow-cljs and ClojureScript unfortunately doesn't support conditional requires like Clojure. At least we got to keep the code to not lint them and the whole idea to start to commit dev-only code inside src/dev/.
Summary
This PR preloads the
user
namespace (src/user.cljs
andsrc/dev/user.cljs
) for themobile
target and for dev-only purposes. The files are git-ignored.Just a reminder that you'll be responsible for making sure your
user
namespace is correct. If it's broken in any way (e.g. calling non-existent code) the app will crash at initialization (dev-only environment obviously).Why?
When the app initializes, it loads namespaces that were required at least once. If you create a
user
namespace, it won't be automatically required for you. And if you, like some Clojure devs, like to use theuser
namespace as your safe heaven for experimentation and dev-only utilities, you'll need to remember to evaluate the namespace at least once.This is tedious and many times I forgot to do so and the app crashed because the compiler didn't know where the symbols were coming from.
How do I use it?
Simply create a file
src/user.cljs
orsrc/dev/user.cljs
and write whatever you want there! Personally, I often use it to persist experiments during development, for example, functions to manipulate the app-db, functions I can call from Emacs directly, debugging functions (e.g. #19906 (comment)), and so on.Some Clojure teams like to commit the
user
namespace and share it with other team members. This can work, but to me it misses the point that I want a namespace that's already loaded and which is my own playground where I don't care about code quality.Areas that may be impacted
None.
Steps to test
Behavior-wise, nothing to test. As a dev, you can verify it works for you by creating a file like the following, and you can require any namespace inside
src/
. Once you load the app, the vars insideuser
ordev.user
ns should be readily available for you to be used anywhere in the repo without having to require theuser
namespace inns
forms.Remember to delete code using the
user
namespace before opening a PR ;)status: ready