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

feat(jsx-one-expression-per-line): add spaceMode option (#241) #248

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

exoRift
Copy link

@exoRift exoRift commented Dec 29, 2023

Description

Adds the spaceMode option to jsx-one-expression-per-line which uses the HTML   entity instead of JSX {' '}

Linked Issues

#241

Additional context

Important

This PR is not ready and requires assistance

There are a few issues:

  • I wasn't able to generate the types.d.ts file (I couldn't figure out how)
  • The test is only replacing the text for the first offending source and not the second offending source. I'm not sure why
  • There's an issue with how this rule parses sources. If you look at the test I added, the first literal has a space preceding the embedded div. When this rule runs, it treats the preceding space as part of the first literal (which doesn't get targeted because it's not a violation) and thus ignores it. For proper fixing, the rule would have to replace that space with   and then add a newline, however, there's no way for me to do that and simply adding   and then a newline creates two spaces. I'm not sure how to resolve this issue
    • In fact, this is an issue with the rule itself even without these changes as the fix keeps that space and then adds a newline with {' '} as well. (Which, again, is two spaces)

Copy link

netlify bot commented Dec 29, 2023

Deploy Preview for eslint-stylistic ready!

Name Link
🔨 Latest commit fb65d71
🔍 Latest deploy log https://app.netlify.com/sites/eslint-stylistic/deploys/658e1c3d6787b000087bb5c3
😎 Deploy Preview https://deploy-preview-248--eslint-stylistic.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@antfu
Copy link
Member

antfu commented Jan 8, 2024

To answer your questions:

  1. You can run npm run update on the project root
  2. That's how rules works, only one change is applied at one time. You could have some basic test for one occurrence (and check the error messages), and then create a new set of tests like
    it('snapshots', async () => {
    const { fix } = createLinter('indent-binary-ops', rule)
    expect(
    fix(unIndent`
    if (
    a && (
    a.b ||
    a.c
    ) &&
    a.d
    ) {}
    `),
    ).toMatchInlineSnapshot(
    `
    "if (
    a && (
    a.b ||
    a.c
    ) &&
    a.d
    ) {}"
    `,
    )
    to test the final result.

@so1ve
Copy link
Contributor

so1ve commented Feb 4, 2024

@exoRift Are you on windows? I met this error: privatenumber/tsx#345 and I had to use WSL to develop

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

Successfully merging this pull request may close these issues.

None yet

3 participants