Skip to content

Commit

Permalink
chore: change the action to use warnings and only fail on new ones (#…
Browse files Browse the repository at this point in the history
…6740)

* chore: change the action to use warnings and only fail on new ones

* chore: test that the CI can fail the check

* chore: test if it correctly prevents bypass

* chore: test that tampering is not allowed

* Revert "chore: test that tampering is not allowed"

This reverts commit 7e71c5f.

* chore: fix first issue in `Editable.tsx`

* chore: ignore workshop files

* chore: decrease max warns

* fix: refactor dynamic icon render to be React Compiler compatible

* fix: make SlateContainer React Compiler ready
  • Loading branch information
stipsan authored May 22, 2024
1 parent 23f6c67 commit ebef5cb
Show file tree
Hide file tree
Showing 8 changed files with 74 additions and 21 deletions.
29 changes: 29 additions & 0 deletions .eslintignore.react-compiler
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
**/etc/*
**/.sanity/*
**/public/*
**/build/*
**/.next/*
**/static/*
**/coverage/*
**/lib/*
**/node_modules/*
**/dist/*
*.json
*.css
*.snap
*.md
dev/test-studio/sanity.theme.mjs
dev/test-studio/workshop/scopes.js
# Ignore paths that are not relevant for the compiler
dev/design-studio/*
dev/embedded-studio/*
dev/starter-cra-studio/*
dev/starter-next-studio/*
dev/starter-studio/*
dev/strict-studio/*
dev/studio-e2e-studio/*
examples/*
perf/*
scripts/*
test/*
**/__workshop__/*
6 changes: 6 additions & 0 deletions .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ const config = {
},
],
// Set react-compiler to `error` once existing issues are fixed
/**
* Once all react-compiler warnings are fixed then this rule should be changed to 'error' and:
* - the `.eslintignore.react-compiler` file should be deleted
* - the `.github/workflows/are-we-compiled-yet.yml` action can be deleted
* - the `pnpm check:react-compiler` command removed
*/
'react-compiler/react-compiler': 'warn',
'react/no-unescaped-entities': 'off',
'i18next/no-literal-string': ['error'],
Expand Down
18 changes: 18 additions & 0 deletions .github/eslint-compact.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"problemMatcher": [
{
"owner": "eslint-compact",
"pattern": [
{
"regexp": "^(.+):\\sline\\s(\\d+),\\scol\\s(\\d+),\\s(Error|Warning|Info)\\s-\\s(.+)\\s\\((.+)\\)\\.?$",
"file": 1,
"line": 2,
"column": 3,
"severity": 4,
"message": 5,
"code": 6
}
]
}
]
}
5 changes: 4 additions & 1 deletion .github/workflows/are-we-compiled-yet.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,8 @@ jobs:
- name: Install project dependencies
run: pnpm install

- name: Register Problem Matcher for ESLint that handles -f compact and shows warnings inline on PRs
run: echo "::add-matcher::.github/eslint-compact.json"

- name: Lint with the React Compiler ESLint plugin
run: pnpm check:react-compiler
run: pnpm check:react-compiler -f compact
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"check:format": "prettier . --check",
"check:lint": "turbo run lint --continue -- --quiet",
"check:react-exhaustive-deps": "turbo run lint --continue -- --quiet --rule 'react-hooks/exhaustive-deps: [error, {additionalHooks: \"(useAsync|useMemoObservable|useObservableCallback)\"}]'",
"check:react-compiler": "turbo run lint --continue -- --quiet --rule 'react-compiler/react-compiler: [error]'",
"check:react-compiler": "eslint --no-inline-config --no-eslintrc --ext .cjs,.mjs,.js,.jsx,.ts,.tsx --parser @typescript-eslint/parser --plugin react-compiler --rule 'react-compiler/react-compiler: [warn]' --ignore-path .eslintignore.react-compiler --max-warnings 46 .",
"check:test": "run-s test -- --silent",
"check:types": "tsc && turbo run check:types --filter='./packages/*' --filter='./packages/@sanity/*'",
"chore:format:fix": "prettier --cache --write .",
Expand Down
10 changes: 7 additions & 3 deletions packages/@sanity/portable-text-editor/src/editor/Editable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -332,10 +332,14 @@ export const PortableTextEditable = forwardRef(function PortableTextEditable(
// Store reference to original apply function (see below for usage in useEffect)
const originalApply = useMemo(() => slateEditor.apply, [slateEditor])

const [syncedRangeDecorations, setSyncedRangeDecorations] = useState(false)
useEffect(() => {
syncRangeDecorations()
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []) // Empty here! We only want to run this once at mount
if (!syncedRangeDecorations) {
// We only want this to run once, on mount
setSyncedRangeDecorations(true)
syncRangeDecorations()
}
}, [syncRangeDecorations, syncedRangeDecorations])

useEffect(() => {
if (!isEqual(rangeDecorations, rangeDecorationsRef.current)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {type PropsWithChildren, useEffect, useMemo} from 'react'
import {type PropsWithChildren, useEffect, useMemo, useState} from 'react'
import {createEditor} from 'slate'
import {Slate, withReact} from 'slate-react'

Expand Down Expand Up @@ -28,8 +28,8 @@ export interface SlateContainerProps extends PropsWithChildren {
export function SlateContainer(props: SlateContainerProps) {
const {patches$, portableTextEditor, readOnly, maxBlocks, keyGenerator} = props

// Create the slate instance
const [slateEditor, subscribe] = useMemo(() => {
// Create the slate instance, using `useState` ensures setup is only run once, initially
const [[slateEditor, subscribe]] = useState(() => {
debug('Creating new Slate editor instance')
const {editor, subscribe: _sub} = withPlugins(withReact(createEditor()), {
keyGenerator,
Expand All @@ -40,9 +40,8 @@ export function SlateContainer(props: SlateContainerProps) {
})
KEY_TO_VALUE_ELEMENT.set(editor, {})
KEY_TO_SLATE_ELEMENT.set(editor, {})
return [editor, _sub]
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []) // Only initial - empty deps here
return [editor, _sub] as const
})

useEffect(() => {
const unsubscribe = subscribe()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,8 @@ const DynamicIconContainer = styled.span`
const accentSpanWrapper = (children: ReactNode) => <AccentSpan>{children}</AccentSpan>

const DynamicIcon = (props: {icon: {url: string}}) => {
const [ref, setRef] = useState<HTMLSpanElement | null>(null)
const [__html, setHtml] = useState('')
useEffect(() => {
if (!ref) return

const controller = new AbortController()
const signal = controller.signal

Expand All @@ -95,23 +93,19 @@ const DynamicIcon = (props: {icon: {url: string}}) => {
}
return response.text()
})
.then((data) => {
if (!ref) return
ref.innerHTML = data
})
.then((data) => setHtml(data))
.catch((error) => {
if (error.name !== 'AbortError') {
console.error(error)
}
})

// eslint-disable-next-line consistent-return
return () => {
controller.abort()
}
}, [ref, props.icon.url])
}, [props.icon.url])

return <DynamicIconContainer ref={setRef} />
return <DynamicIconContainer dangerouslySetInnerHTML={{__html}} />
}

function NormalBlock(props: {children: ReactNode}) {
Expand Down

0 comments on commit ebef5cb

Please sign in to comment.