refactor: organize worker namespaces and lint them

Organize them to live under src/main/frontend/worker and add a lint to ensure
that common code with frontend is only under frontend/common/.
Add a linter to ensure that worker doesn't depend on frontend.
Motivated to fix after recent worker breakage fixed by
75463c4df4
This commit is contained in:
Gabriel Horner
2024-08-08 22:32:28 -04:00
parent b62715ebd8
commit 7a40761eca
28 changed files with 92 additions and 54 deletions

View File

@@ -81,7 +81,7 @@ frontend.ui/_emoji-init-data
;; placeholder var for defonce
frontend.worker.rtc.op-mem-layer/_sync-loop-canceler
;; Used by shadow.cljs
frontend.db-worker/init
frontend.worker.db-worker/init
;; WIP fn, remove when it's ready
frontend.worker.rtc.asset-sync/<loop-for-assets-sync
;; Future use?

View File

@@ -197,7 +197,7 @@
frontend.test.helper/with-reset cljs.test/async
frontend.worker.rtc.idb-keyval-mock/with-reset-idb-keyval-mock cljs.test/async
frontend.react/defc clojure.core/defn
frontend.schema-register/defkeyword cljs.spec.alpha/def}
frontend.common.schema-register/defkeyword cljs.spec.alpha/def}
:skip-comments true
:output {:progress true
:exclude-files ["src/test/docs-0.10.9/"]}}

View File

@@ -120,6 +120,9 @@ jobs:
- name: Lint invalid translation entries
run: bb lang:validate-translations
- name: Lint to keep worker independent of frontend
run: bb lint:worker-and-frontend-separate
- name: Lint to keep db and file graph code separate
run: bb lint:db-and-file-graphs-separate

9
bb.edn
View File

@@ -139,9 +139,6 @@
dev:validate-ast
logseq.tasks.malli/validate-ast
dev:lint
logseq.tasks.dev/lint
dev:test
logseq.tasks.dev/test
@@ -151,6 +148,9 @@
dev:gen-malli-kondo-config
logseq.tasks.dev/gen-malli-kondo-config
lint:dev
logseq.tasks.dev.lint/dev
lint:large-vars
logseq.bb-tasks.lint.large-vars/-main
@@ -163,6 +163,9 @@
lint:db-and-file-graphs-separate
logseq.tasks.dev.db-and-file-graphs/-main
lint:worker-and-frontend-separate
logseq.tasks.dev.lint/worker-and-frontend-separate
nbb:watch
logseq.bb-tasks.nbb.watch/watch

View File

@@ -7,7 +7,7 @@ This page describes development practices for this codebase.
Most of our linters require babashka. Before running them, please [install babashka](https://github.com/babashka/babashka#installation). To invoke all the linters in this section, run
```sh
bb dev:lint
bb lint:dev
```
### Clojure code
@@ -120,6 +120,15 @@ $ bb lint:db-and-file-graphs-separate
The main convention is that file and db specific files go under directories named `file_based` and `db_based` respectively. To see the full list of file and db specific namespaces and files see the top of [the script](/scripts/src/logseq/tasks/dev/db_and_file_graphs.clj).
### Separate Worker from Frontend
The worker and frontend code share common code from deps/ and `frontend.common.*`. However, the worker should never depend on other frontend namespaces as it could pull in libraries like React which cause it to fail hard. Run this linter to ensure worker namespaces are not dependent on other frontend namespaces:
```
$ bb lint:worker-and-frontend-separate
Success!
```
## Testing
We have unit, performance and end to end tests.

View File

@@ -5,28 +5,12 @@
[babashka.fs :as fs]
[babashka.cli :as cli]
[logseq.tasks.util :as task-util]
[logseq.tasks.dev.lint :as dev-lint]
[clojure.java.io :as io]
[clojure.pprint :as pp]
[clojure.edn :as edn]
[clojure.data :as data]))
(defn lint
"Run all lint tasks
- clj-kondo lint
- carve lint for unused vars
- lint for vars that are too large
- lint invalid translation entries
- lint to ensure file and db graph remain separate"
[]
(doseq [cmd ["clojure -M:clj-kondo --parallel --lint src --cache false"
"bb lint:carve"
"bb lint:large-vars"
"bb lint:db-and-file-graphs-separate"
"bb lang:validate-translations"
"bb lint:ns-docstrings"]]
(println cmd)
(shell cmd)))
(defn test
"Run tests. Pass args through to cmd 'yarn cljs:run-test'"
[& args]
@@ -37,10 +21,9 @@
"Run all lint tasks, then run tests(exclude testcases tagged by :long).
pass args through to cmd 'yarn cljs:run-test'"
[]
(lint)
(dev-lint/dev)
(test "-e" "long" "-e" "fix-me"))
(defn gen-malli-kondo-config
"Generate clj-kondo type-mismatch config from malli schema
.clj-kondo/metosin/malli-types/config.edn"

View File

@@ -0,0 +1,39 @@
(ns logseq.tasks.dev.lint
(:require [clojure.string :as string]
[babashka.process :refer [shell]]))
(defn dev
"Run all lint tasks
- clj-kondo lint
- carve lint for unused vars
- lint for vars that are too large
- lint invalid translation entries
- lint to ensure file and db graph remain separate
- lint to ensure worker and frontend are separate"
[]
(doseq [cmd ["clojure -M:clj-kondo --parallel --lint src --cache false"
"bb lint:carve"
"bb lint:large-vars"
"bb lint:worker-and-frontend-separate"
"bb lint:db-and-file-graphs-separate"
"bb lang:validate-translations"
"bb lint:ns-docstrings"]]
(println cmd)
(shell cmd)))
(defn worker-and-frontend-separate
"Ensures worker is independent of frontend"
[]
(let [res (shell {:out :string}
"git grep -h" "\\[frontend.*:as" "src/main/frontend/worker")
req-lines (->> (:out res)
string/split-lines
(remove #(re-find #"frontend\.worker|frontend\.common" %)))]
(if (seq req-lines)
(do
(println "The following requires should not be in worker namespaces:")
(println (string/join "\n" req-lines))
(System/exit 1))
(println "Success!"))))

View File

@@ -30,7 +30,7 @@
{:entries [frontend.extensions.tldraw]
:depends-on #{:main}}
:db-worker
{:init-fn frontend.db-worker/init
{:init-fn frontend.worker.db-worker/init
:depends-on #{:shared}
:web-worker true}}
@@ -118,7 +118,7 @@
{:entries [frontend.extensions.tldraw]
:depends-on #{:main}}
:db-worker
{:init-fn frontend.db-worker/init
{:init-fn frontend.worker.db-worker/init
:depends-on #{:shared}
:web-worker true}}

View File

@@ -1,5 +1,5 @@
(ns frontend.common.missionary-util
"Utils based on missionary."
"Utils based on missionary. Used by frontend and worker namespaces"
(:require-macros [frontend.common.missionary-util])
(:require [clojure.core.async :as a]
[missionary.core :as m])

View File

@@ -1,5 +1,6 @@
(ns frontend.schema-register
"Macro 'defkeyword' to def keyword with docstring and malli-schema")
(ns frontend.common.schema-register
"Macro 'defkeyword' to def keyword with docstring and malli-schema.
Used by frontend and worker namespaces")
(defmacro defkeyword
@@ -8,5 +9,5 @@
(assert (keyword? kw) "must be keyword")
(assert (some? docstring) "must have 'docstring' arg")
(when optional-malli-schema
`(do (assert (frontend.schema-register/not-register-yet? ~kw) (str "Already registered: " ~kw))
(frontend.schema-register/register! ~kw ~optional-malli-schema))))
`(do (assert (frontend.common.schema-register/not-register-yet? ~kw) (str "Already registered: " ~kw))
(frontend.common.schema-register/register! ~kw ~optional-malli-schema))))

View File

@@ -1,4 +1,4 @@
(ns frontend.schema-register
(ns frontend.common.schema-register
"Set malli default registry to a mutable one,
and use `register!` to add schemas dynamically."
(:require [malli.core :as m]

View File

@@ -1,5 +1,5 @@
(ns frontend.search.fuzzy
"fuzzy search"
(ns frontend.common.search-fuzzy
"fuzzy search. Used by frontend and worker namespaces"
(:require [clojure.string :as string]
[cljs-bean.core :as bean]
[frontend.worker.util :as worker-util]))

View File

@@ -1,6 +1,6 @@
(ns frontend.common-keywords
"There are some keywords scattered throughout the codebase."
(:require [frontend.schema-register :include-macros true :as sr]))
(:require [frontend.common.schema-register :include-macros true :as sr]))
(sr/defkeyword :block/uuid

View File

@@ -13,7 +13,7 @@
[frontend.date :as date]
[frontend.db :as db]
[frontend.handler.property :as property-handler]
[frontend.search.fuzzy :as fuzzy-search]
[frontend.common.search-fuzzy :as fuzzy]
[frontend.state :as state]
[frontend.ui :as ui]
[frontend.util :as util]
@@ -749,8 +749,8 @@
(defn- fuzzy-matched?
[input s]
(pos? (fuzzy-search/score (string/lower-case (str input))
(string/lower-case (str s)))))
(pos? (fuzzy/score (string/lower-case (str input))
(string/lower-case (str s)))))
(defn- row-matched?
[row input filters]

View File

@@ -11,7 +11,7 @@
[frontend.log]
[frontend.page :as page]
[frontend.routes :as routes]
[frontend.schema-register :as sr]
[frontend.common.schema-register :as sr]
[frontend.spec]
[logseq.api]
[malli.dev.cljs :as md]
@@ -49,7 +49,7 @@
(defn ^:export start []
(when config/dev?
(md/start!))
(frontend.schema-register/init)
(frontend.common.schema-register/init)
(when-let [node (.getElementById js/document "root")]
(set-router!)
(rum/mount (page/current-page) node)

View File

@@ -7,7 +7,7 @@
[frontend.state :as state]
[frontend.util :as util]
[promesa.core :as p]
[frontend.search.fuzzy :as fuzzy]
[frontend.common.search-fuzzy :as fuzzy]
[logseq.common.config :as common-config]
[frontend.db.async :as db-async]
[frontend.db :as db]

View File

@@ -8,7 +8,7 @@
[frontend.worker.util :as worker-util]
[promesa.core :as p]
[logseq.outliner.batch-tx :as batch-tx]
[frontend.schema-register :as sr]))
[frontend.common.schema-register :as sr]))
(defn- entity-datoms=>attr->datom

View File

@@ -1,4 +1,4 @@
(ns frontend.db-worker
(ns frontend.worker.db-worker
"Worker used for browser DB implementation"
(:require ["@logseq/sqlite-wasm" :default sqlite3InitModule]
["comlink" :as Comlink]

View File

@@ -1,7 +1,7 @@
(ns frontend.worker.pipeline
"Pipeline work after transaction"
(:require [datascript.core :as d]
[frontend.schema-register :as sr]
[frontend.common.schema-register :as sr]
[frontend.worker.file :as file]
[frontend.worker.react :as worker-react]
[frontend.worker.state :as worker-state]

View File

@@ -1,7 +1,7 @@
(ns frontend.worker.rtc.asset-db-listener
"Listen asset-block changes in db, generate asset-sync operations"
(:require [datascript.core :as d]
[frontend.schema-register :as sr]
[frontend.common.schema-register :as sr]
[frontend.worker.db-listener :as db-listener]
[frontend.worker.rtc.asset :as r.asset]
[frontend.worker.rtc.client-op :as client-op]))

View File

@@ -2,7 +2,7 @@
"listen datascript changes, infer operations from the db tx-report"
(:require [clojure.string :as string]
[datascript.core :as d]
[frontend.schema-register :include-macros true :as sr]
[frontend.common.schema-register :include-macros true :as sr]
[frontend.worker.db-listener :as db-listener]
[frontend.worker.rtc.client-op :as client-op]
[logseq.db :as ldb]))

View File

@@ -1,6 +1,6 @@
(ns frontend.worker.rtc.exception
"Exception list"
(:require [frontend.schema-register :as sr]))
(:require [frontend.common.schema-register :as sr]))
(sr/defkeyword :rtc.exception/remote-graph-not-exist
"Remote exception. e.g. push client-updates to a deleted graph.")

View File

@@ -1,6 +1,6 @@
(ns frontend.worker.rtc.log-and-state
"Fns to generate rtc related logs"
(:require [frontend.schema-register :as sr]
(:require [frontend.common.schema-register :as sr]
[frontend.worker.util :as worker-util]
[malli.core :as ma]
[missionary.core :as m]))

View File

@@ -4,7 +4,7 @@
[clojure.set :as set]
[clojure.string :as string]
[datascript.core :as d]
[frontend.schema-register :as sr]
[frontend.common.schema-register :as sr]
[frontend.worker.handler.page :as worker-page]
[frontend.worker.rtc.client-op :as client-op]
[frontend.worker.rtc.const :as rtc-const]

View File

@@ -6,7 +6,7 @@
["fuse.js" :as fuse]
[goog.object :as gobj]
[datascript.core :as d]
[frontend.search.fuzzy :as fuzzy]
[frontend.common.search-fuzzy :as fuzzy]
[frontend.worker.util :as worker-util]
[logseq.db.sqlite.util :as sqlite-util]
[logseq.common.util :as common-util]

View File

@@ -2,7 +2,7 @@
"State hub for worker"
(:require [logseq.common.util :as common-util]
[logseq.common.config :as common-config]
[frontend.schema-register :include-macros true :as sr]))
[frontend.common.schema-register :include-macros true :as sr]))
(sr/defkeyword :undo/repo->page-block-uuid->undo-ops
"{repo {<page-block-uuid> [op1 op2 ...]}}")

View File

@@ -2,7 +2,7 @@
"undo/redo related fns and op-schema"
(:require [clojure.set :as set]
[datascript.core :as d]
[frontend.schema-register :include-macros true :as sr]
[frontend.common.schema-register :include-macros true :as sr]
[logseq.outliner.batch-tx :include-macros true :as batch-tx]
[frontend.worker.db-listener :as db-listener]
[frontend.worker.state :as worker-state]

View File

@@ -4,7 +4,7 @@
[frontend.worker.db-listener :as db-listener]
[frontend.worker.state :as worker-state]
[clojure.set :as set]
[frontend.schema-register :include-macros true :as sr]
[frontend.common.schema-register :include-macros true :as sr]
[malli.core :as m]
[malli.util :as mu]
[logseq.db :as ldb]))