Skip to content

Commit

Permalink
Shared CLJ/CLJS lib (#14980)
Browse files Browse the repository at this point in the history
* Shared CLJ/CLJS lib (PoC/WIP)

* PoC 2.0

* Fixes 🔧

* More test fixes 🔧

* Bump shadow-cljs version

* Fix more stuff

* Need to ^:export the exports

* CI fixes 🔧

* Add eslintignore

* Ignore cljs files for FE code coverage

* Try prefixing CLJS -> JS import with goog:

* Revert indentation change

* No goog:

* Add .prettierignore

* Use advanced build for now for JS tests unit we can figure out how to make it work
  • Loading branch information
camsaul committed Mar 5, 2021
1 parent f4d90f6 commit cca03d2
Show file tree
Hide file tree
Showing 17 changed files with 140 additions and 90 deletions.
4 changes: 2 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -492,11 +492,11 @@ jobs:
# .BACKEND-CHECKSUMS is every Clojure source file as well as dependency files like deps.edn and plugin manifests
- create-checksum-file:
filename: .BACKEND-CHECKSUMS
find-args: ". -type f -name '*.clj' -or -name '*.java' -or -name '*.edn' -or -name '*.yaml' -or -name sample-dataset.db.mv.db"
find-args: ". -type f -name '*.clj' -or -name '*.cljc' -or -name '*.java' -or -name '*.edn' -or -name '*.yaml' -or -name sample-dataset.db.mv.db"
# .FRONTEND-CHECKSUMS is every JavaScript source file as well as dependency files like yarn.lock
- create-checksum-file:
filename: .FRONTEND-CHECKSUMS
find-args: ". -type f -name '*.js' -or -name '*.jsx' -or -name '*.json' -or -name yarn.lock -or -name sample-dataset.db.mv.db"
find-args: ". -type f -name '*.js' -or -name '*.jsx' -or -name '*.cljc' -or -name '*.json' -or -name yarn.lock -or -name sample-dataset.db.mv.db"
# .MODULES-CHECKSUMS is every Clojure source file in the modules/ directory as well as plugin manifests
- create-checksum-file:
filename: .MODULES-CHECKSUMS
Expand Down
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
frontend/src/cljs
1 change: 1 addition & 0 deletions .github/workflows/frontend.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ on:
- '**'
paths:
- 'frontend/**'
- 'shared/**'
- 'enterprise/frontend/**'
- 'docs/**'
- '**/package.lock'
Expand Down
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -74,5 +74,6 @@ xcshareddata
xcuserdata
dev/src/dev/nocommit/
*~

**/cypress_sample_dataset.json
/frontend/src/cljs
.shadow-cljs
1 change: 1 addition & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
frontend/src/cljs
9 changes: 9 additions & 0 deletions bin/build-mb/src/build.clj
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@
(u/announce "CI run: enforce the lockfile")
(u/sh {:dir u/project-root-directory} "yarn" "--frozen-lockfile"))
(u/sh {:dir u/project-root-directory} "yarn")))
;; TODO -- I don't know why it doesn't work if we try to combine the two steps below by calling `yarn build`,
;; which does the same thing.
(u/step "Build frontend (ClojureScript)"
(u/sh {:dir u/project-root-directory
:env {"PATH" (env/env :path)
"HOME" (env/env :user-home)
"NODE_ENV" "production"
"MB_EDITION" mb-edition}}
"./node_modules/.bin/shadow-cljs" "release" "app"))
(u/step "Run 'webpack' with NODE_ENV=production to assemble and minify frontend assets"
(u/sh {:dir u/project-root-directory
:env {"PATH" (env/env :path)
Expand Down
44 changes: 2 additions & 42 deletions frontend/src/metabase/lib/types.js
Original file line number Diff line number Diff line change
@@ -1,46 +1,6 @@
import _ from "underscore";
import { isa, TYPE } from "cljs/metabase.types";

import MetabaseSettings from "metabase/lib/settings";

const PARENTS = MetabaseSettings.get("types");

/// Basically exactly the same as Clojure's isa?
/// Recurses through the type hierarchy until it can give you an answer.
/// isa(TYPE.BigInteger, TYPE.Number) -> true
/// isa(TYPE.Text, TYPE.Boolean) -> false
export function isa(child, ancestor) {
if (!child || !ancestor) {
return false;
}

if (child === ancestor) {
return true;
}

const parents = PARENTS[child];
if (!parents) {
if (child !== "type/*") {
console.error("Invalid type:", child);
} // the base type is the only type with no parents, so anything else that gets here is invalid
return false;
}

for (const parent of parents) {
if (isa(parent, ancestor)) {
return true;
}
}

return false;
}

// build a pretty sweet dictionary of top-level types, so people can do TYPE.Latitude instead of "type/Latitude" and get error messages / etc.
// this should also make it easier to keep track of things when we tweak the type hierarchy
export const TYPE = {};
for (const type of _.keys(PARENTS)) {
const key = type.substring(5); // strip off "type/"
TYPE[key] = type;
}
export { isa, TYPE };

// convenience functions since these operations are super-common
// this will also make it easier to tweak how these checks work in the future,
Expand Down
3 changes: 2 additions & 1 deletion jest.unit.conf.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
"collectCoverageFrom": ["frontend/src/**/*.js", "frontend/src/**/*.jsx"],
"coveragePathIgnorePatterns": [
"/node_modules/",
"/frontend/src/metabase/visualizations/lib/errors.js"
"/frontend/src/metabase/visualizations/lib/errors.js",
"/frontend/src/cljs/"
]
}
25 changes: 17 additions & 8 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@
"promise-loader": "^1.0.0",
"raf": "^3.4.0",
"react-test-renderer": "~16.14.0",
"shadow-cljs": "2.11.20",
"style-loader": "^0.19.0",
"uglifyjs-webpack-plugin": "^1.0.0",
"unused-files-webpack-plugin": "^3.0.0",
Expand All @@ -153,22 +154,30 @@
"yaml-lint": "^1.2.4"
},
"scripts": {
"dev": "concurrently --kill-others -p name -n 'backend,frontend,docs' -c 'blue,green,yellow' 'lein run' 'yarn build-hot' 'yarn docs'",
"dev-ee": "concurrently --kill-others -p name -n 'backend,frontend,docs' -c 'blue,green,yellow' 'lein with-profiles +ee run' 'MB_EDITION=ee yarn build-hot' 'yarn docs'",
"concurrently": "yarn && concurrently --kill-others -p name",
"dev": "yarn concurrently -n 'backend,frontend,cljs,docs' -c 'blue,green,yellow,magenta' 'lein run' 'yarn build-hot:js' 'yarn build-hot:cljs' 'yarn docs'",
"dev-ee": "yarn concurrently -n 'backend,frontend,cljs,docs' -c 'blue,green,yellow,magenta' 'lein with-profiles +ee run' 'MB_EDITION=ee yarn build-hot:js' 'MB_EDITION=ee yarn build-hot:cljs' 'yarn docs'",
"lint": "yarn lint-eslint && yarn lint-prettier && yarn lint-docs-links && yarn lint-yaml",
"lint-eslint": "yarn && eslint --ext .js --ext .jsx --rulesdir frontend/lint/eslint-rules --max-warnings 0 enterprise/frontend/src frontend/src enterprise/frontend/test frontend/test",
"lint-eslint": "yarn build-quick:cljs && eslint --ext .js --ext .jsx --rulesdir frontend/lint/eslint-rules --max-warnings 0 enterprise/frontend/src frontend/src enterprise/frontend/test frontend/test",
"lint-prettier": "yarn && prettier -l '{enterprise/,}frontend/**/*.{js,jsx,css}' || (echo '\nThese files are not formatted correctly. Did you forget to \"yarn prettier\"?' && false)",
"lint-docs-links": "yarn && ./bin/verify-doc-links",
"lint-yaml": "yamllint **/*.{yaml,yml} --ignore=node_modules/**/*.{yaml,yml}",
"test": "yarn test-unit && yarn test-timezones && yarn test-cypress",
"test-unit": "yarn && jest --maxWorkers=2 --config jest.unit.conf.json",
"test-unit": "yarn build-quick:cljs && jest --maxWorkers=2 --config jest.unit.conf.json",
"test-unit-watch": "yarn test-unit --watch",
"test-unit-update-snapshot": "yarn test-unit --updateSnapshot",
"test-timezones-unit": "yarn && jest --maxWorkers=2 --config jest.tz.unit.conf.json",
"test-timezones-unit": "yarn build-quick:cljs && jest --maxWorkers=2 --config jest.tz.unit.conf.json",
"test-timezones": "yarn && ./frontend/test/__runner__/run_timezone_tests",
"build": "yarn && webpack --bail",
"build-watch": "yarn && webpack --watch",
"build-hot": "yarn && NODE_ENV=hot webpack-dev-server --progress",
"build:js": "yarn && webpack --bail",
"build-watch:js": "yarn && webpack --watch",
"build-hot:js": "yarn && NODE_ENV=hot webpack-dev-server --progress",
"build:cljs": "yarn && shadow-cljs release app",
"build-quick:cljs": "yarn build:cljs",
"build-watch:cljs": "yarn && shadow-cljs watch app",
"build-hot:cljs": "yarn && shadow-cljs watch app",
"build": "yarn build:cljs && yarn build:js",
"build-watch": "yarn concurrently -n 'cljs,js' 'yarn build-watch:cljs' 'yarn build-watch:js'",
"build-hot": "yarn concurrently -n 'cljs,js' 'yarn build-hot:cljs' 'yarn build-hot:js'",
"build-stats": "yarn && webpack --json > stats.json",
"build-shared": "yarn && webpack --config webpack.shared.config.js",
"start": "yarn build && lein ring server",
Expand Down
8 changes: 4 additions & 4 deletions project.clj
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@
["-target" "1.8", "-source" "1.8"]

:source-paths
["src" "backend/mbql/src"]
["src" "backend/mbql/src" "shared/src"]

:java-source-paths
["java"]
Expand All @@ -193,7 +193,7 @@

:dev
{:source-paths ["dev/src" "local/src"]
:test-paths ["test" "backend/mbql/test"]
:test-paths ["test" "backend/mbql/test" "shared/test"]

:dependencies
[[clj-http-fake "1.0.3" :exclusions [slingshot]] ; Library to mock clj-http responses
Expand Down Expand Up @@ -398,8 +398,8 @@
[:test-common
{:dependencies [[camsaul/cloverage "1.2.1.1" :exclusions [riddley]]]
:plugins [[camsaul/lein-cloverage "1.2.1.1"]]
:source-paths ^:replace ["src" "backend/mbql/src" "enterprise/backend/src"]
:test-paths ^:replace ["test" "backend/mbql/test" "enterprise/backend/test"]
:source-paths ^:replace ["src" "backend/mbql/src" "enterprise/backend/src" "shared/src"]
:test-paths ^:replace ["test" "backend/mbql/test" "enterprise/backend/test" "shared/test"]
:cloverage {:fail-threshold 69
:exclude-call
[;; don't instrument logging forms, since they won't get executed as part of tests anyway
Expand Down
12 changes: 12 additions & 0 deletions shadow-cljs.edn
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
;; shadow-cljs configuration
{:source-paths
["shared/src"]

:dependencies
[]

:builds
{:app {:target :npm-module
:output-dir "frontend/src/cljs/"
;; empty entries = export all namespaces (?)
:entries []}}}
14 changes: 14 additions & 0 deletions shared/src/metabase/shared/util.cljc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
(ns metabase.shared.util)

(defn qualified-name
"Return `k` as a string, qualified by its namespace, if any (unlike `name`). Handles `nil` values gracefully as well
(also unlike `name`).
(u/qualified-name :type/FK) -> \"type/FK\""
[k]
(when (some? k)
(if-let [namespac (when #?(:clj (instance? clojure.lang.Named k)
:cljs (keyword? k))
(namespace k))]
(str namespac "/" (name k))
(name k))))
19 changes: 17 additions & 2 deletions src/metabase/types.clj → shared/src/metabase/types.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
which in turn derive from their own parents. This makes it possible to add new types without needing to add
corresponding mappings in the frontend or other places. For example, a Database may want a type called something
like `:type/CaseInsensitiveText`; we can add this type as a derivative of `:type/Text` and everywhere else can
continue to treat it as such until further notice.")
continue to treat it as such until further notice."
(:require [metabase.shared.util :as shared.u]))

;; NOTE: be sure to update frontend/test/metabase-bootstrap.js when updating this

Expand Down Expand Up @@ -210,7 +211,7 @@

;;; ---------------------------------------------------- Util Fns ----------------------------------------------------

(defn types->parents
(defn- types->parents
"Return a map of various types to their parent types.
This is intended for export to the frontend as part of `MetabaseBootstrap` so it can build its own implementation of
Expand All @@ -226,3 +227,17 @@
{:arglists '([field])}
[{base-type :base_type, semantic-type :semantic_type}]
(some #(isa? % :type/Temporal) [base-type semantic-type]))

#?(:cljs
(defn ^:export isa
"Is `x` the same as, or a descendant type of, `y`?"
[x y]
(isa? (keyword x) (keyword y))))

#?(:cljs
(def ^:export TYPE
"A map of Type name (as string, without `:type/` namespace) -> qualified type name as string
{\"Temporal\" \"type/Temporal\", ...}"
(clj->js (into {} (for [tyype (descendants :type/*)]
[(name tyype) (shared.u/qualified-name tyype)])))))
13 changes: 0 additions & 13 deletions src/metabase/public_settings.clj
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
[metabase.models.setting :as setting :refer [defsetting]]
[metabase.plugins.classloader :as classloader]
[metabase.public-settings.metastore :as metastore]
[metabase.types :as types]
[metabase.util :as u]
[metabase.util.i18n :as i18n :refer [available-locales-with-names deferred-tru trs tru]]
[metabase.util.password :as password]
Expand Down Expand Up @@ -323,18 +322,6 @@
:setter :none
:getter driver.u/available-drivers-info)

(defsetting types
"Field types"
:visibility :public
:setter :none
:getter (fn [] (types/types->parents :type/*)))

(defsetting entities
"Entity types"
:visibility :public
:setter :none
:getter (fn [] (types/types->parents :entity/*)))

(defsetting has-sample-dataset?
"Whether this instance has a Sample Dataset database"
:visibility :authenticated
Expand Down
20 changes: 8 additions & 12 deletions src/metabase/util.clj
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
[flatland.ordered.map :refer [ordered-map]]
[medley.core :as m]
[metabase.config :as config]
[metabase.shared.util :as shared.u]
[metabase.util.i18n :refer [trs tru]]
[potemkin :as p]
[ring.util.codec :as codec]
[weavejester.dependency :as dep])
(:import [java.net InetAddress InetSocketAddress Socket]
Expand All @@ -23,6 +25,12 @@
javax.xml.bind.DatatypeConverter
[org.apache.commons.validator.routines RegexValidator UrlValidator]))

(comment shared.u/keep-me)

(p/import-vars
[shared.u
qualified-name])

(defn format-bytes
"Nicely format `num-bytes` as kilobytes/megabytes/etc.
Expand Down Expand Up @@ -463,18 +471,6 @@
[f coll]
(into {} (map (juxt f identity)) coll))

(defn qualified-name
"Return `k` as a string, qualified by its namespace, if any (unlike `name`). Handles `nil` values gracefully as well
(also unlike `name`).
(u/qualified-name :type/FK) -> \"type/FK\""
[k]
(when (some? k)
(if-let [namespac (when (instance? clojure.lang.Named k)
(namespace k))]
(str namespac "/" (name k))
(name k))))

(defn id
"If passed an integer ID, returns it. If passed a map containing an `:id` key, returns the value if it is an integer.
Otherwise returns `nil`.
Expand Down
8 changes: 5 additions & 3 deletions webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const LIB_SRC_PATH = __dirname + "/frontend/src/metabase-lib";
const ENTERPRISE_SRC_PATH =
__dirname + "/enterprise/frontend/src/metabase-enterprise";
const TYPES_SRC_PATH = __dirname + "/frontend/src/metabase-types";
const CLJS_SRC_PATH = __dirname + "/frontend/src/cljs";
const TEST_SUPPORT_PATH = __dirname + "/frontend/test/__support__";
const BUILD_PATH = __dirname + "/resources/frontend_client";

Expand Down Expand Up @@ -67,12 +68,12 @@ const config = (module.exports = {
rules: [
{
test: /\.(js|jsx)$/,
exclude: /node_modules/,
exclude: /node_modules|cljs/,
use: [{ loader: "babel-loader", options: BABEL_CONFIG }],
},
{
test: /\.(js|jsx)$/,
exclude: /node_modules|\.spec\.js/,
exclude: /node_modules|cljs|\.spec\.js/,
use: [
{
loader: "eslint-loader",
Expand Down Expand Up @@ -108,6 +109,7 @@ const config = (module.exports = {
"metabase-lib": LIB_SRC_PATH,
"metabase-enterprise": ENTERPRISE_SRC_PATH,
"metabase-types": TYPES_SRC_PATH,
"cljs": CLJS_SRC_PATH,
__support__: TEST_SUPPORT_PATH,
style: SRC_PATH + "/css/core/index",
ace: __dirname + "/node_modules/ace-builds/src-min-noconflict",
Expand Down Expand Up @@ -210,7 +212,7 @@ if (NODE_ENV === "hot") {
config.module.rules.unshift({
test: /\.jsx$/,
// NOTE: our verison of react-hot-loader doesn't play nice with react-dnd's DragLayer, so we exclude files named `*DragLayer.jsx`
exclude: /node_modules|DragLayer\.jsx$/,
exclude: /node_modules|cljs|DragLayer\.jsx$/,
use: [
// NOTE Atte Keinänen 10/19/17: We are currently sticking to an old version of react-hot-loader
// because newer versions would require us to upgrade to react-router v4 and possibly deal with
Expand Down
Loading

0 comments on commit cca03d2

Please sign in to comment.