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

refactor: eslint #1053

Merged
merged 20 commits into from
Feb 14, 2025
Merged

refactor: eslint #1053

merged 20 commits into from
Feb 14, 2025

Conversation

kellyjosephprice
Copy link
Collaborator

@kellyjosephprice kellyjosephprice commented Feb 4, 2025

PR App Fix RM-11916

🧰 Changes

Gets eslint working in the repo again.

There's some code changes, but hopefully the behavior is the same. I've also moved test only docs from ./docs/ to ./__tests/fixtures/ to make it clearer.

TODO

There's still a ton of old tests that need to either be updated or removed.

🧬 QA & Testing

Comment on lines -11 to -24
it.skip('Variables', () => {
expect(mdx(mdast('<<variable:user>>'))).toMatchInlineSnapshot(`
"<<variable:user>>
"
`);
});

it.skip('Glossary Items', () => {
expect(mdx(mdast('<<glossary:owl>>'))).toMatchInlineSnapshot(`
"<<glossary:owl>>
"
`);
});

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Obsolete syntax

@@ -1,17 +0,0 @@
import { mdast, mdx } from '../..';

describe.skip('break compiler', () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This node type doesn't exist anymore.

@@ -10,4 +10,4 @@ exports[`Components > Embed 3`] = `"<div class="embed embed_hasImg"><a class="em

exports[`Components > Embed 4`] = `"<div class="embed "><a class="embed-link" href="https://www.nytimes.com/2020/05/03/us/politics/george-w-bush-coronavirus-unity.html" rel="noopener noreferrer" target="_blank"><div class="embed-body"><small class="embed-provider">nytimes.com</small><div class="embed-title">rdmd</div></div></a></div>"`;

exports[`Components > Image 1`] = `"<span aria-label="Expand image" class="img lightbox closed" role="button" tabindex="0"><span class="lightbox-inner"><img src="https://files.readme.io/6f52e22-man-eating-pizza-and-making-an-ok-gesture.jpg" width="auto" height="auto" title="Pizza Face" class="img " alt="Bro eats pizza and makes an OK gesture." loading="lazy"></span></span>"`;
exports[`Components > Image 1`] = `"<span aria-label="Expand image" class="img lightbox closed" role="button" tabindex="0"><span class="lightbox-inner"><img alt="Bro eats pizza and makes an OK gesture." class="img " height="auto" loading="lazy" src="https://files.readme.io/6f52e22-man-eating-pizza-and-making-an-ok-gesture.jpg" title="Pizza Face" width="auto"></span></span>"`;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The attributes are sorted now.

spy = vi.spyOn(console, prop).mockImplementation(impl);
spy = vi.spyOn(console, prop);
// @ts-expect-error - spy is a spy
spy.mockImplementation(impl);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure what's going on here with the types.

Copy link
Member

Choose a reason for hiding this comment

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

It's partially because of the undefined type on spy and also because the spyOn is being done in a try-catch. This diff will fix it up.

diff --git a/__tests__/helpers.ts b/__tests__/helpers.ts
index 71fdd9f8..0fa9507d 100644
--- a/__tests__/helpers.ts
+++ b/__tests__/helpers.ts
@@ -1,3 +1,5 @@
+import type { MockInstance } from 'vitest';
+
 import * as rdmd from '@readme/markdown-legacy';
 
 import { vi } from 'vitest';
@@ -7,13 +9,10 @@ import { run, compile, migrate as baseMigrate } from '../index';
 export const silenceConsole =
   (prop: keyof Console = 'error', impl = () => {}) =>
   fn => {
-    let spy: ReturnType<typeof vi.spyOn> | undefined;
+    const spy: MockInstance = vi.spyOn(console, prop);
 
     try {
-      spy = vi.spyOn(console, prop);
-      // @ts-expect-error - spy is a spy
       spy.mockImplementation(impl);
-
       return fn(spy);
     } finally {
       spy?.mockRestore();
``

@@ -210,52 +210,6 @@ test.skip('should render nothing if nothing passed in', () => {
expect(_html('')).toBeNull();
});

test.skip('`correctnewlines` option', () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These options aren't used any more.

@@ -31,7 +31,7 @@ describe('migrating html blocks', () => {
const mdx = migrate(md);
expect(mdx).toMatchInlineSnapshot(`
"<HTMLBlock>{\`
<a href="example.com">${'\\`example.com\\`'}</a>
Copy link
Collaborator Author

@kellyjosephprice kellyjosephprice Feb 12, 2025

Choose a reason for hiding this comment

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

Nested levels of string parsing twists my brain.

@kellyjosephprice kellyjosephprice changed the title refactor: docs and tests refactor: eslint Feb 12, 2025
@kellyjosephprice kellyjosephprice marked this pull request as ready for review February 13, 2025 23:59
spy = vi.spyOn(console, prop).mockImplementation(impl);
spy = vi.spyOn(console, prop);
// @ts-expect-error - spy is a spy
spy.mockImplementation(impl);
Copy link
Member

Choose a reason for hiding this comment

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

It's partially because of the undefined type on spy and also because the spyOn is being done in a try-catch. This diff will fix it up.

diff --git a/__tests__/helpers.ts b/__tests__/helpers.ts
index 71fdd9f8..0fa9507d 100644
--- a/__tests__/helpers.ts
+++ b/__tests__/helpers.ts
@@ -1,3 +1,5 @@
+import type { MockInstance } from 'vitest';
+
 import * as rdmd from '@readme/markdown-legacy';
 
 import { vi } from 'vitest';
@@ -7,13 +9,10 @@ import { run, compile, migrate as baseMigrate } from '../index';
 export const silenceConsole =
   (prop: keyof Console = 'error', impl = () => {}) =>
   fn => {
-    let spy: ReturnType<typeof vi.spyOn> | undefined;
+    const spy: MockInstance = vi.spyOn(console, prop);
 
     try {
-      spy = vi.spyOn(console, prop);
-      // @ts-expect-error - spy is a spy
       spy.mockImplementation(impl);
-
       return fn(spy);
     } finally {
       spy?.mockRestore();
``

Comment on lines +23 to +24
// @ts-expect-error - the custom matcher types are not being set up
// correctly
Copy link
Member

Choose a reason for hiding this comment

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

This appears to be happening because the expect here is coming from @types/jest and the custom matcher is being set up on the expect that Vitest offers. We can clean this up when we excise Jest from this after moving puppeteer tests over to Vitest browser testing.

const code = await mdx.compile(doc, { ...opts, components: componentsByExport, useTailwind: true });
const content = await mdx.run(code, { components: executedComponents, terms, variables });

setError(() => null);
setContent(() => content);
} catch (e) {
// eslint-disable-next-line no-console
console.error(e);
Copy link
Member

Choose a reason for hiding this comment

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

would love if the markdown library had a pluggable logging system

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What are you imagining?

@kellyjosephprice kellyjosephprice merged commit e93a1f6 into next Feb 14, 2025
14 checks passed
@kellyjosephprice kellyjosephprice deleted the refactor/docs-and-tests branch February 14, 2025 18:43
rafegoldberg pushed a commit that referenced this pull request Feb 18, 2025
## Version 8.3.0
### ✨ New & Improved

* eslint ([#1053](#1053)) ([e93a1f6](e93a1f6))

### 🛠 Fixes & Updates

* pass through props ([#1057](#1057)) ([613ae3c](613ae3c))

<!--SKIP CI-->
@rafegoldberg
Copy link
Contributor

This PR was released!

🚀 Changes included in v8.3.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants