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

Remove JSX tag whitelist; use case instead #1551

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
177 changes: 177 additions & 0 deletions npm-jsx-list-lowercase-tags/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
#!/usr/bin/env node

var fs = require('fs');

var types = require('ast-types');
var esprima = require('esprima-fb');

var namedTypes = types.namedTypes;

var knownTags = {
a: true,
abbr: true,
address: true,
applet: true,
area: true,
article: true,
aside: true,
audio: true,
b: true,
base: true,
bdi: true,
bdo: true,
big: true,
blockquote: true,
body: true,
br: true,
button: true,
canvas: true,
caption: true,
circle: true,
cite: true,
code: true,
col: true,
colgroup: true,
command: true,
data: true,
datalist: true,
dd: true,
defs: true,
del: true,
details: true,
dfn: true,
dialog: true,
div: true,
dl: true,
dt: true,
ellipse: true,
em: true,
embed: true,
fieldset: true,
figcaption: true,
figure: true,
footer: true,
form: true,
g: true,
h1: true,
h2: true,
h3: true,
h4: true,
h5: true,
h6: true,
head: true,
header: true,
hgroup: true,
hr: true,
html: true,
i: true,
iframe: true,
img: true,
input: true,
ins: true,
kbd: true,
keygen: true,
label: true,
legend: true,
li: true,
line: true,
linearGradient: true,
link: true,
main: true,
map: true,
mark: true,
marquee: true,
mask: false,
menu: true,
menuitem: true,
meta: true,
meter: true,
nav: true,
noscript: true,
object: true,
ol: true,
optgroup: true,
option: true,
output: true,
p: true,
param: true,
path: true,
pattern: false,
polygon: true,
polyline: true,
pre: true,
progress: true,
q: true,
radialGradient: true,
rect: true,
rp: true,
rt: true,
ruby: true,
s: true,
samp: true,
script: true,
section: true,
select: true,
small: true,
source: true,
span: true,
stop: true,
strong: true,
style: true,
sub: true,
summary: true,
sup: true,
svg: true,
table: true,
tbody: true,
td: true,
text: true,
textarea: true,
tfoot: true,
th: true,
thead: true,
time: true,
title: true,
tr: true,
track: true,
tspan: true,
u: true,
ul: true,
'var': true,
video: true,
wbr: true
};

process.argv.slice(2).forEach(function(filename) {
fs.readFile(filename, 'utf8', function(err, code) {
if (err) {
console.log(err.message);
process.exit(1);
}

if (code.indexOf('@jsx') === -1) {
return;
}

try {
var ast = esprima.parse(code, {loc: true});
} catch (e) {
process.stderr.write(
'\x1B[31mError parsing ' + filename + ': ' + e + '\x1B[0m\n'
);
return;
}

types.traverse(ast, function(node) {
if (namedTypes.XJSOpeningElement.check(node) &&
/^[a-z]/.test(node.name.name) &&
!knownTags.hasOwnProperty(node.name.name)) {
console.log(
filename + ':' +
node.loc.start.line + ':' +
code.split('\n')[node.loc.start.line - 1]
);
}
});
});
});
15 changes: 15 additions & 0 deletions npm-jsx-list-lowercase-tags/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"name": "jsx-list-lowercase-tags",
"version": "0.1.0",
"description": "List lowercase user-defined JSX tags found in JavaScript files.",
"preferGlobal": true,
"bin": {
"jsx-list-lowercase-tags": "./index.js"
},
"author": "",
"license": "Apache-2.0",
"dependencies": {
"esprima-fb": "~3001.1.0-dev-harmony-fb",
"ast-types": "~0.3.28"
}
}
3 changes: 1 addition & 2 deletions vendor/fbtransform/transforms/react.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
var Syntax = require('esprima-fb').Syntax;
var utils = require('jstransform/src/utils');

var FALLBACK_TAGS = require('./xjs').knownTags;
var renderXJSExpressionContainer =
require('./xjs').renderXJSExpressionContainer;
var renderXJSLiteral = require('./xjs').renderXJSLiteral;
Expand Down Expand Up @@ -62,7 +61,7 @@ function visitReactTag(traverse, object, path, state) {
'Namespace tags are not supported. ReactJSX is not XML.');
}

var isFallbackTag = FALLBACK_TAGS.hasOwnProperty(nameObject.name);
var isFallbackTag = /^[a-z]/.test(nameObject.name);
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this also apply it for member expressions?

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 need to test though it sounds like @sebmarkbage is moving some stuff around here anyway now that #2287 is in. We decided that tags with - and : are always treated as native tags (i.e., strings) and member expressions (with .) are always JS references, regardless of capitalization.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sound 👍

PS. Although I'd check for member expression and not for ..

utils.append(
(isFallbackTag ? jsxObjIdent + '.' : '') + (nameObject.name) + '(',
state
Expand Down
136 changes: 0 additions & 136 deletions vendor/fbtransform/transforms/xjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,141 +18,6 @@
var Syntax = require('esprima-fb').Syntax;
var utils = require('jstransform/src/utils');

var knownTags = {
a: true,
abbr: true,
address: true,
applet: true,
area: true,
article: true,
aside: true,
audio: true,
b: true,
base: true,
bdi: true,
bdo: true,
big: true,
blockquote: true,
body: true,
br: true,
button: true,
canvas: true,
caption: true,
circle: true,
cite: true,
code: true,
col: true,
colgroup: true,
command: true,
data: true,
datalist: true,
dd: true,
defs: true,
del: true,
details: true,
dfn: true,
dialog: true,
div: true,
dl: true,
dt: true,
ellipse: true,
em: true,
embed: true,
fieldset: true,
figcaption: true,
figure: true,
footer: true,
form: true,
g: true,
h1: true,
h2: true,
h3: true,
h4: true,
h5: true,
h6: true,
head: true,
header: true,
hgroup: true,
hr: true,
html: true,
i: true,
iframe: true,
img: true,
input: true,
ins: true,
kbd: true,
keygen: true,
label: true,
legend: true,
li: true,
line: true,
linearGradient: true,
link: true,
main: true,
map: true,
mark: true,
marquee: true,
mask: false,
menu: true,
menuitem: true,
meta: true,
meter: true,
nav: true,
noscript: true,
object: true,
ol: true,
optgroup: true,
option: true,
output: true,
p: true,
param: true,
path: true,
pattern: false,
polygon: true,
polyline: true,
pre: true,
progress: true,
q: true,
radialGradient: true,
rect: true,
rp: true,
rt: true,
ruby: true,
s: true,
samp: true,
script: true,
section: true,
select: true,
small: true,
source: true,
span: true,
stop: true,
strong: true,
style: true,
sub: true,
summary: true,
sup: true,
svg: true,
table: true,
tbody: true,
td: true,
text: true,
textarea: true,
tfoot: true,
th: true,
thead: true,
time: true,
title: true,
tr: true,
track: true,
tspan: true,
u: true,
ul: true,
'var': true,
video: true,
wbr: true
};

function renderXJSLiteral(object, isLast, state, start, end) {
var lines = object.value.split(/\r\n|\n|\r/);

Expand Down Expand Up @@ -246,7 +111,6 @@ function trimLeft(value) {
return value.replace(/^[ ]+/, '');
}

exports.knownTags = knownTags;
exports.renderXJSExpressionContainer = renderXJSExpressionContainer;
exports.renderXJSLiteral = renderXJSLiteral;
exports.quoteAttrName = quoteAttrName;
Expand Down