Skip to content
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

And more lint fixes #44

Merged
merged 4 commits into from
Jan 23, 2025
Merged

And more lint fixes #44

merged 4 commits into from
Jan 23, 2025

Conversation

tstirrat15
Copy link
Contributor

Description

Fixes a bunch of lint issues around the editors

Changes

  • Pin the monaco editor using ~ instead of ^. The latter lets minor versions float, which we don't want with a zero-versioned library.
  • Lint fixes

Testing

Review. See that things are still green. See that the editor still works as expected.

Copy link

vercel bot commented Jan 22, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 22, 2025 11:50pm

Copy link
Contributor Author

@tstirrat15 tstirrat15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments

package.json Outdated
@@ -31,7 +31,7 @@
"lodash.throttle": "^4.1.1",
"markdown-it": "^13.0.1",
"marked": "^4.0.10",
"monaco-editor-core": "^0.39.0",
"monaco-editor-core": "~0.40.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This keeps the editor on the same version.

@@ -31,7 +31,7 @@
"lodash.throttle": "^4.1.1",
"markdown-it": "^13.0.1",
"marked": "^4.0.10",
"monaco-editor-core": "^0.39.0",
"monaco-editor": "~0.40.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Near as I can tell, @monaco-editor/react depends on monaco-editor, not -core, which means that using it should both give us more faithful typing and also doesn't require bringing in a separate library.

@@ -52,7 +52,7 @@ export type EditorDisplayProps = {
diff?: string | undefined;
themeName?: string | undefined;
hideMinimap?: boolean | undefined;
fontSize?: string | undefined;
fontSize?: number | undefined;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were passing "13px" to a prop that takes a number. This fixes that.

message: `Malformed or invalid test data relationship: ${invalid.parsed.errorMessage}`,
severity: monacoRef.MarkerSeverity.Error,
});
if (monacoRef) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typeguarding

@@ -14,7 +15,7 @@ export const TUPLE_THEME_NAME = 'tuple-theme';
export const TUPLE_DARK_THEME_NAME = 'tuple-theme-dark';

export default function registerTupleLanguage(
monaco: any,
monaco: typeof monacoEditor,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This type is cribbed from the typing of useMonaco, so this now matches what the function actually receives.


comments: {
lineComment: '//',
blockComment: ['/*', '*/'],
blockComment: ['/*', '*/'] satisfies [string, string],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are typed as tuples, but typescript treats the literals as string[]. Using satisfies treats them as such without whacking the type.

// Based on: https://github.com/microsoft/monaco-languages/blob/main/src/typescript/typescript.ts
const richEditConfiguration = {

wordPattern:
/(-?\d*\.\d\w*)|([^\`\~\!\@\#\%\^\&\*\(\)\-\=\+\[\{\]\}\\\|\;\:\'\"\,\.\<\>\/\?\s]+)/g,
/(-?\d*\.\d\w*)|([^`~!@#%^&*()\-=+[{\]}\\|;:'",.<>/?\s]+)/g,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these were marked as unnecessary escapes by eslint.

@tstirrat15 tstirrat15 merged commit 0ca6bc3 into main Jan 23, 2025
4 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jan 23, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants