Skip to content

Commit

Permalink
Merge pull request #2378 from opral/lint-light
Browse files Browse the repository at this point in the history
MessageLintReports get() and getAll() async, with no reactivity, to reduce excessive resource consumption
  • Loading branch information
jldec authored Apr 5, 2024
2 parents db0c0ee + b5fef9f commit ea473bc
Show file tree
Hide file tree
Showing 22 changed files with 267 additions and 250 deletions.
12 changes: 12 additions & 0 deletions .changeset/rich-mirrors-look.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
"@inlang/sdk": minor
"@inlang/badge": patch
"@inlang/cli": patch
"@inlang/editor": patch
"@inlang/github-lint-action": patch
"vs-code-extension": patch
"@inlang/sdk-load-test": patch
---

SDK Breaking change: LintReports get() and getAll() are now async, and non-reactive.
Reduces (does not eliminate) excessive sdk resource consumption.
21 changes: 4 additions & 17 deletions inlang/guides/how-to-inlang-app/how-to-inlang-app.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,28 +76,15 @@ These functions internally handle the loading and saving of messages according t

[Lint rules](/c/lint-rules) are crucial for maintaining code quality and consistency. In your Inlang app, you can leverage lint rules to ensure that your localization messages adhere to specific standards. For example, let's explore working with the [`snakeCaseId`](https://inlang.com/m/messageLintRule.inlang.snakeCaseId) lint rule.

This lint rule checks whether your message id's are in a snale case format or not.
This lint rule checks whether your message id's are in a snake case format or not.

To access lint reports based on the configured lint rule on a specific `message`, you can subscribe on `inlang.query.messageLintReports.get.subscribe()` – we introduced a subscription pattern here because of the nature of [lint rules](/c/lint-rules) which can frequently update, like with every update of the `message`.

To get the lint reports:
To fetch lint reports for a message, use the async lintReports.get() function.

```typescript
inlang.query.messageLintReports.get.subscribe(
{
where: {
messageId: message.messageId,
},
},
(reports) => {
/* do something with reports */
}
);
const reports = await inlang.query.lintReports.get({ where: { messageId: message.id } })
/* do something with reports */
```

In this example, we subscribe to lint reports for a specific message using `message.messageId` and can now execute a specific action with the `reports` in the callback.


## Publishing your app

If you are ready and want to release what you have built, please [publish your app](https://inlang.com/documentation/publish-to-marketplace) on our [Marketplace](https://inlang.com) – where you can also sell apps in the future.
Expand Down
38 changes: 2 additions & 36 deletions inlang/source-code/badge/src/badge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { readFileSync } from "node:fs"
import { removeCommas } from "./helper/removeCommas.js"
import { calculateSummary } from "./helper/calculateSummary.js"
import { caching } from "cache-manager"
import { type MessageLintReport, loadProject } from "@inlang/sdk"
import { loadProject } from "@inlang/sdk"

const fontMedium = readFileSync(new URL("../assets/static/Inter-Medium.ttf", import.meta.url))
const fontBold = readFileSync(new URL("../assets/static/Inter-Bold.ttf", import.meta.url))
Expand Down Expand Up @@ -53,41 +53,7 @@ export const badge = async (urlQuery: string, projectQuery?: string) => {

const settings = project.settings()

// TODO: async reports
const MessageLintReportsAwaitable = (): Promise<MessageLintReport[]> => {
return new Promise((resolve) => {
let reports = project.query.messageLintReports.getAll()

if (reports) {
// reports where loaded
setTimeout(() => {
// this is a workaround. We do not know when the report changed. Normally this shouldn't be a issue for cli
const newReports = project.query.messageLintReports.getAll()
if (newReports) {
resolve(newReports)
}
}, 200)
} else {
let counter = 0
const interval = setInterval(() => {
reports = project.query.messageLintReports.getAll()
if (reports) {
clearInterval(interval)
resolve(reports)
} else {
counter += 1
}

if (counter > 30) {
clearInterval(interval)
resolve([])
}
}, 200)
}
})
}

const reports = await MessageLintReportsAwaitable()
const reports = await project.query.messageLintReports.getAll()

const { percentage, errors, warnings, numberOfMissingVariants } = calculateSummary({
reports: reports,
Expand Down
38 changes: 2 additions & 36 deletions inlang/source-code/cli/src/commands/lint/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Command } from "commander"
import Table from "cli-table3"
import { getInlangProject } from "../../utilities/getInlangProject.js"
import { log } from "../../utilities/log.js"
import { LanguageTag, type InlangProject, type MessageLintReport } from "@inlang/sdk"
import { LanguageTag, type InlangProject } from "@inlang/sdk"
import { projectOption } from "../../utilities/globalFlags.js"

export const lint = new Command()
Expand All @@ -29,41 +29,7 @@ export async function lintCommandAction(args: { project: InlangProject; logger:

const languageTags: LanguageTag[] = options.languageTags ? options.languageTags.split(",") : []

// TODO: async reports
const MessageLintReportsAwaitable = (): Promise<MessageLintReport[]> => {
return new Promise((resolve) => {
let reports = args.project.query.messageLintReports.getAll()

if (reports) {
// reports where loaded
setTimeout(() => {
// this is a workaround. We do not know when the report changed. Normally this shouldn't be a issue for cli
const newReports = args.project.query.messageLintReports.getAll()
if (newReports) {
resolve(newReports)
}
}, 200)
} else {
let counter = 0
const interval = setInterval(() => {
reports = args.project.query.messageLintReports.getAll()
if (reports) {
clearInterval(interval)
resolve(reports)
} else {
counter += 1
}

if (counter > 10) {
clearInterval(interval)
resolve([])
}
}, 200)
}
})
}

let reports = await MessageLintReportsAwaitable()
let reports = await args.project.query.messageLintReports.getAll()

if (reports.length === 0) {
args.logger.success("Linting successful.")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createEffect, createSignal, For, on, Show } from "solid-js"
import { createResource, createEffect, createSignal, For, on, Show } from "solid-js"
import { useEditorState } from "./State.jsx"
import { createVisibilityObserver } from "@solid-primitives/intersection-observer"
import { PatternEditor } from "./components/PatternEditor.jsx"
Expand All @@ -14,7 +14,7 @@ export function Message(props: { id: string }) {
const { project, filteredLanguageTags, filteredIds, filteredMessageLintRules, textSearch } =
useEditorState()
const [message, setMessage] = createSignal<MessageType>()
const [lintReports, setLintReports] = createSignal<Readonly<MessageLintReport[]>>([])
const [lintReports, setLintReports] = createSignal<readonly MessageLintReport[]>([])
const [shouldMessageBeShown, setShouldMessageBeShown] = createSignal(true)
// const [blockChangeMessageIsFocused, setBlockChangeMessageIsFocused] = createSignal<Date>(new Date())

Expand All @@ -33,34 +33,40 @@ export function Message(props: { id: string }) {
}
})

createEffect(() => {
if (!project.loading) {
project()!.query.messages.get.subscribe({ where: { id: props.id } }, (message) =>
setMessage(message)
)
}
})
// TODO: Find out why messages.get() in solidAdapter only works with subscribe.
// by contrast, query.messages.getAll() signal does work. see ListHeader.tsx
createEffect(
on([project], () => {
if (!project.loading) {
project()!.query.messages.get.subscribe({ where: { id: props.id } }, (message) => {
setMessage(message)
})
}
})
)

createEffect(() => {
if (!project.loading && message()?.id) {
project()!.query.messageLintReports.get.subscribe(
{ where: { messageId: message()!.id } },
(report) => {
if (report) {
setLintReports(report)
setHasBeenLinted(true)
}
}
)
// createResource re-fetches lintReports via async api whenever the message changes
createResource(message, async (message) => {
if (!project.loading && message?.id) {
const reports = await project()!.query.messageLintReports.get({
where: { messageId: message.id },
})
// set lintReports synchronously for setShouldMessageBeShown effect on setHasBeenLinted
// createResource waits for next tick to update itself using the return value
setLintReports(reports)
// trigger hasBeenLinted signal once, the first time the message is linted
setHasBeenLinted(true)
return reports
}
return []
})

createEffect(
on(
[filteredLanguageTags, filteredMessageLintRules, filteredIds, textSearch, hasBeenLinted],
() => {
setShouldMessageBeShown((prev) => {
const result = !showFilteredMessage(message())
const result = !showFilteredMessage(message(), lintReports())
// check if message count changed and update the global message count
if (result !== prev && result === true) {
setMessageCount((prev) => prev - 1)
Expand Down Expand Up @@ -168,7 +174,7 @@ export function Message(props: { id: string }) {
<PatternEditor
languageTag={languageTag}
message={message()!}
lintReports={lintReports() as MessageLintReport[]}
lintReports={(lintReports() || []) as MessageLintReport[]}
// hidden={!(filteredLanguageTags().includes(languageTag) || filteredLanguageTags().length === 0)}
/>
</Show>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useEditorState } from "../State.jsx"
import { For, Show, createMemo } from "solid-js"
import { For, Show, createSignal, createEffect, createMemo, createResource } from "solid-js"
import { TourHintWrapper } from "./Notification/TourHintWrapper.jsx"
import IconAdd from "~icons/material-symbols/add"
import { type InstalledMessageLintRule, type MessageLintRule } from "@inlang/sdk"
Expand All @@ -16,13 +16,28 @@ export const ListHeader = () => {
tourStep,
} = useEditorState()

const [getMessages, setMessages] = createSignal()

createEffect(() => {
if (!project.loading) {
const messages = project()!.query.messages.getAll()
setMessages(messages)
}
})

// createResource re-fetches lintReports via async api whenever messages change
const [lintReports] = createResource(getMessages, async () => {
const reports = await project()!.query.messageLintReports.getAll()
return reports
})

const getLintSummary = createMemo(() => {
const summary = new Map<string, number>() // Use a Map with explicit types for better performance
const filteredRules = new Set<string>(filteredMessageLintRules())
const filteredTags = new Set<string>(filteredLanguageTags())
const filteredIdsValue = filteredIds()

for (const report of project()?.query.messageLintReports.getAll() || []) {
for (const report of lintReports() || []) {
const ruleId = report.ruleId
const languageTag = report.languageTag
const messageId = report.messageId
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import type { MessageLintReport, Message } from "@inlang/sdk"
import { useEditorState } from "../State.jsx"
export const showFilteredMessage = (message: Message | undefined) => {
export const showFilteredMessage = (
message: Message | undefined,
lintReports: readonly MessageLintReport[] | undefined
) => {
const { filteredMessageLintRules, filteredLanguageTags, filteredIds, textSearch, project } =
useEditorState()

Expand Down Expand Up @@ -61,12 +64,10 @@ export const showFilteredMessage = (message: Message | undefined) => {
const filteredByLintRules =
lintRulesSet.size === 0 ||
(message !== undefined &&
project()!
.query.messageLintReports.get({ where: { messageId: message.id } })
?.some(
(report: MessageLintReport) =>
lintRulesSet.has(report.ruleId) && languageTagsSet.has(report.languageTag)
))
lintReports?.some(
(report: MessageLintReport) =>
lintRulesSet.has(report.ruleId) && languageTagsSet.has(report.languageTag)
))
? filteredBySearch
: false

Expand Down
4 changes: 2 additions & 2 deletions inlang/source-code/github-lint-action/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export async function run(): Promise<void> {
continue
}
result.installedRules.push(...projectBase.installed.messageLintRules())
result.reportsBase.push(...projectBase.query.messageLintReports.getAll())
result.reportsBase.push(...(await projectBase.query.messageLintReports.getAll()))
}

// Collect meta data for head and base repository
Expand Down Expand Up @@ -148,7 +148,7 @@ export async function run(): Promise<void> {
result.installedRules.push(newRule)
}
}
result?.reportsHead.push(...projectHead.query.messageLintReports.getAll())
result?.reportsHead.push(...(await projectHead.query.messageLintReports.getAll()))
}

// Create a lint summary for each project
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,16 @@ export async function linterDiagnostics(args: { context: vscode.ExtensionContext
}) ?? state().project.query.messages.getByDefaultAlias(message.messageId)

if (_message) {
state().project.query.messageLintReports.get.subscribe(
state().project.query.messages.get.subscribe(
{
where: {
messageId: _message.id,
id: _message.id,
},
},
(reports) => {
async () => {
const reports = await state().project.query.messageLintReports.get({
where: { messageId: _message.id },
})
const diagnostics: vscode.Diagnostic[] = []

if (!reports) {
Expand Down
Loading

0 comments on commit ea473bc

Please sign in to comment.