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

Relax JSX pragma regexp #2643

Merged

Conversation

gfmio
Copy link
Contributor

@gfmio gfmio commented May 17, 2020

Fixes #2642.

This PR relaxes the JSX pragma detection slightly.

Previously, only pragmas at the beginning of docstring comments were recognised. But pragmas can occur anywhere in any block comment (not just docstring comments). This PR fixes the regexp that recognises pragmas.

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Can we add some test cases to cover this change?

@gfmio
Copy link
Contributor Author

gfmio commented May 23, 2020

I actually just realised that the previous pragma matcher has a bug anyway, as it would recognise line comments of the form //* @jsx myPragma as valid even though they're not. Scratch that, I just tested it and Babel accepts pragmas in line comments (TypeScript does not)

@gfmio
Copy link
Contributor Author

gfmio commented May 23, 2020

@ljharb I've added the tests

@gfmio gfmio requested a review from ljharb May 23, 2020 09:17
lib/util/pragma.js Outdated Show resolved Hide resolved
lib/util/pragma.js Outdated Show resolved Hide resolved

const assert = require('assert');
const SourceCode = require('eslint').SourceCode;
const espree = require('espree');
Copy link
Member

Choose a reason for hiding this comment

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

is there any way to do this with eslint itself, rather than adding a dev dep on espree?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not as far as I could tell, unfortunately. I tried using eslint first, but eslint does not expose the API to generate the AST object needed to create the SourceCode object.

@gfmio gfmio requested a review from ljharb June 12, 2020 10:28
@ljharb ljharb force-pushed the bugfix/2642-jsx-pragma-matching-too-strict branch from d2ce572 to ef9a512 Compare June 12, 2020 20:51
@ljharb ljharb merged commit ef9a512 into jsx-eslint:master Jun 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

JSX pragma matching too strict
2 participants