Skip to content

Commit d605864

Browse files
thepasslethepassle
authored
Merge pull request #1 from open-wc/fix/jsdoc-safety-guard
fix: add jsdoc safety guard, and improve logging
2 parents 59ef00d + 75cab3d commit d605864

File tree

12 files changed

+90
-76
lines changed

12 files changed

+90
-76
lines changed

‎packages/analyzer/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ cem analyze
3838
| analyze | | Analyze your components | |
3939
| --globs | string[] | Globs to analyze | `--globs "foo.js"` |
4040
| --exclude | string[] | Globs to exclude | `--exclude "foo.js"` |
41+
| --dev | boolean | Enables extra logging for debugging | `--dev` |
4142
| --litelement | boolean | Enable special handling for LitElement syntax | `--litelement` |
4243
| --fast | boolean | Enable special handling for FASTElement syntax | `--fast` |
4344
| --stencil | boolean | Enable special handling for Stencil syntax | `--stencil` |

‎packages/analyzer/browser/create.js

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,18 @@ var analyzer = (function (exports, ts) {
3333
}
3434
}
3535

36+
/**
37+
* TS seems to struggle sometimes with the `.getText()` method on JSDoc annotations, like `@deprecated` in ts v4.0.0 and `@override` in ts v4.3.2
38+
* This is a bug in TS, but still annoying, so we add some safety rails here
39+
*/
40+
const safe = (cb, returnType = '') => {
41+
try {
42+
return cb();
43+
} catch {
44+
return returnType;
45+
}
46+
};
47+
3648
/**
3749
* UTILITIES RELATED TO MODULE IMPORTS
3850
*/
@@ -330,7 +342,7 @@ var analyzer = (function (exports, ts) {
330342
* @example @attr
331343
*/
332344
function hasAttrAnnotation(member) {
333-
return member?.jsDoc?.some(jsDoc => jsDoc?.tags?.some(tag => tag?.tagName?.getText() === 'attr'));
345+
return member?.jsDoc?.some(jsDoc => jsDoc?.tags?.some(tag => safe(() => tag?.tagName?.getText()) === 'attr'));
334346
}
335347

336348

@@ -494,7 +506,7 @@ var analyzer = (function (exports, ts) {
494506

495507

496508
/** @summary */
497-
if(tag?.tagName?.getText() === 'summary') {
509+
if(safe(() => tag?.tagName?.getText()) === 'summary') {
498510
doc.summary = tag.comment;
499511
}
500512

@@ -1666,7 +1678,7 @@ var analyzer = (function (exports, ts) {
16661678
* Instead, we use TS for this JSDoc annotation.
16671679
*/
16681680
jsDoc?.tags?.forEach(tag => {
1669-
switch(tag?.tagName?.getText()) {
1681+
switch(safe(() => tag?.tagName?.getText())) {
16701682
case 'summary':
16711683
classDoc.summary = tag?.comment;
16721684
break;
@@ -2161,7 +2173,6 @@ var analyzer = (function (exports, ts) {
21612173
/**
21622174
* 🚨 TODO
21632175
* - Lightning web components
2164-
* - storybook
21652176
*/
21662177

21672178
/**
@@ -2170,7 +2181,7 @@ var analyzer = (function (exports, ts) {
21702181
* This function is the core of the analyzer. It takes an array of ts sourceFiles, and creates a
21712182
* custom elements manifest.
21722183
*/
2173-
function create({modules, plugins = []}) {
2184+
function create({modules, plugins = [], dev = false}) {
21742185
const customElementsManifest = {
21752186
schemaVersion: '0.1.0',
21762187
readme: '',
@@ -2185,6 +2196,7 @@ var analyzer = (function (exports, ts) {
21852196
const context = {};
21862197

21872198
modules.forEach(currModule => {
2199+
if(dev) console.log('[COLLECT PHASE]: ', currModule.fileName);
21882200
/**
21892201
* COLLECT PHASE
21902202
* First pass through all modules. Can be used to gather imports, exports, types, default values,
@@ -2194,6 +2206,7 @@ var analyzer = (function (exports, ts) {
21942206
});
21952207

21962208
modules.forEach(currModule => {
2209+
if(dev) console.log('[ANALYZE PHASE]: ', currModule.fileName);
21972210
const moduleDoc = {
21982211
kind: "javascript-module",
21992212
path: currModule.fileName,

‎packages/analyzer/browser/lit.js

Lines changed: 42 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,47 @@ var lit = (function (exports, ts) {
55

66
var ts__default = /*#__PURE__*/_interopDefaultLegacy(ts);
77

8+
/**
9+
* GENERAL UTILITIES
10+
*/
11+
12+
const has = arr => Array.isArray(arr) && arr.length > 0;
13+
14+
/**
15+
* @example node?.decorators?.find(decorator('Component'))
16+
*/
17+
const decorator = type => decorator => decorator?.expression?.expression?.getText() === type || decorator?.expression?.getText() === type;
18+
19+
function resolveModuleOrPackageSpecifier(moduleDoc, name) {
20+
const foundImport = moduleDoc?.imports?.find(_import => _import.name === name);
21+
22+
/* item is imported from another file */
23+
if(foundImport) {
24+
if(foundImport.isBareModuleSpecifier) {
25+
/* import is from 3rd party package */
26+
return { package: foundImport.importPath }
27+
} else {
28+
/* import is imported from a local module */
29+
return { module: new URL(foundImport.importPath, `file:///${moduleDoc.path}`).pathname }
30+
}
31+
} else {
32+
/* item is in current module */
33+
return { module: moduleDoc.path }
34+
}
35+
}
36+
37+
/**
38+
* TS seems to struggle sometimes with the `.getText()` method on JSDoc annotations, like `@deprecated` in ts v4.0.0 and `@override` in ts v4.3.2
39+
* This is a bug in TS, but still annoying, so we add some safety rails here
40+
*/
41+
const safe = (cb, returnType = '') => {
42+
try {
43+
return cb();
44+
} catch {
45+
return returnType;
46+
}
47+
};
48+
849
/**
950
* Whether or not node is:
1051
* - Number
@@ -47,35 +88,6 @@ var lit = (function (exports, ts) {
4788
}
4889
}
4990

50-
/**
51-
* GENERAL UTILITIES
52-
*/
53-
54-
const has = arr => Array.isArray(arr) && arr.length > 0;
55-
56-
/**
57-
* @example node?.decorators?.find(decorator('Component'))
58-
*/
59-
const decorator = type => decorator => decorator?.expression?.expression?.getText() === type || decorator?.expression?.getText() === type;
60-
61-
function resolveModuleOrPackageSpecifier(moduleDoc, name) {
62-
const foundImport = moduleDoc?.imports?.find(_import => _import.name === name);
63-
64-
/* item is imported from another file */
65-
if(foundImport) {
66-
if(foundImport.isBareModuleSpecifier) {
67-
/* import is from 3rd party package */
68-
return { package: foundImport.importPath }
69-
} else {
70-
/* import is imported from a local module */
71-
return { module: new URL(foundImport.importPath, `file:///${moduleDoc.path}`).pathname }
72-
}
73-
} else {
74-
/* item is in current module */
75-
return { module: moduleDoc.path }
76-
}
77-
}
78-
7991
/**
8092
* CUSTOMELEMENT
8193
*
@@ -334,7 +346,7 @@ var lit = (function (exports, ts) {
334346

335347

336348
/** @summary */
337-
if(tag?.tagName?.getText() === 'summary') {
349+
if(safe(() => tag?.tagName?.getText()) === 'summary') {
338350
doc.summary = tag.comment;
339351
}
340352

‎packages/analyzer/custom-elements.json

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,36 +9,22 @@
99
{
1010
"kind": "class",
1111
"description": "",
12-
"name": "MyElement",
12+
"name": "Foo",
1313
"members": [
1414
{
15-
"kind": "field",
16-
"name": "message",
17-
"default": "'bar'",
18-
"description": "- some description",
19-
"type": {
20-
"text": "string"
21-
}
15+
"kind": "method",
16+
"name": "shouldSubscribe",
17+
"description": "Determines whether the element should attempt to subscribe i.e. begin querying\nOverride to prevent subscribing unless your conditions are met"
2218
}
23-
],
24-
"attributes": [
25-
{
26-
"description": "description goes here",
27-
"name": "my-attr"
28-
}
29-
],
30-
"superclass": {
31-
"name": "HTMLElement"
32-
},
33-
"customElement": true
19+
]
3420
}
3521
],
3622
"exports": [
3723
{
3824
"kind": "js",
39-
"name": "MyElement",
25+
"name": "Foo",
4026
"declaration": {
41-
"name": "MyElement",
27+
"name": "Foo",
4228
"module": "fixtures/-default/package/bar.js"
4329
}
4430
}

‎packages/analyzer/fixtures/-default/package/bar.js

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,8 @@
1-
2-
3-
/**
4-
* @attr my-attr description goes here
5-
*/
6-
export class MyElement extends HTMLElement {
7-
message = ''
8-
9-
constructor() {
10-
super();
11-
/** @type {string} - some description */
12-
this.message = 'bar';
13-
}
1+
export class Foo {
2+
/**
3+
* Determines whether the element should attempt to subscribe i.e. begin querying
4+
* Override to prevent subscribing unless your conditions are met
5+
* @override
6+
*/
7+
shouldSubscribe() {}
148
}

‎packages/analyzer/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ import {
6868
const customElementsManifest = create({
6969
modules,
7070
plugins,
71+
dev: mergedOptions.dev
7172
});
7273

7374
fs.writeFileSync(`${process.cwd()}/custom-elements.json`, `${JSON.stringify(customElementsManifest, null, 2)}\n`);

‎packages/analyzer/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@custom-elements-manifest/analyzer",
3-
"version": "0.1.10",
3+
"version": "0.1.11",
44
"description": "",
55
"license": "MIT",
66
"type": "module",

‎packages/analyzer/src/create.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import { FEATURES } from './features/index.js';
44
/**
55
* 🚨 TODO
66
* - Lightning web components
7-
* - storybook
87
*/
98

109
/**
@@ -13,7 +12,7 @@ import { FEATURES } from './features/index.js';
1312
* This function is the core of the analyzer. It takes an array of ts sourceFiles, and creates a
1413
* custom elements manifest.
1514
*/
16-
export function create({modules, plugins = []}) {
15+
export function create({modules, plugins = [], dev = false}) {
1716
const customElementsManifest = {
1817
schemaVersion: '0.1.0',
1918
readme: '',
@@ -28,6 +27,7 @@ export function create({modules, plugins = []}) {
2827
const context = {};
2928

3029
modules.forEach(currModule => {
30+
if(dev) console.log('[COLLECT PHASE]: ', currModule.fileName);
3131
/**
3232
* COLLECT PHASE
3333
* First pass through all modules. Can be used to gather imports, exports, types, default values,
@@ -37,6 +37,7 @@ export function create({modules, plugins = []}) {
3737
});
3838

3939
modules.forEach(currModule => {
40+
if(dev) console.log('[ANALYZE PHASE]: ', currModule.fileName);
4041
const moduleDoc = {
4142
kind: "javascript-module",
4243
path: currModule.fileName,

‎packages/analyzer/src/features/analyse-phase/class-jsdoc.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import parse from 'comment-parser';
22
import { handleJsDocType } from '../../utils/jsdoc.js';
3+
import { safe } from '../../utils/index.js';
34

45
/**
56
* CLASS-JSDOC
@@ -102,7 +103,7 @@ export function classJsDocPlugin() {
102103
* Instead, we use TS for this JSDoc annotation.
103104
*/
104105
jsDoc?.tags?.forEach(tag => {
105-
switch(tag?.tagName?.getText()) {
106+
switch(safe(() => tag?.tagName?.getText())) {
106107
case 'summary':
107108
classDoc.summary = tag?.comment;
108109
break;

‎packages/analyzer/src/features/analyse-phase/creators/handlers.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import ts from 'typescript';
2-
import { has, resolveModuleOrPackageSpecifier } from '../../../utils/index.js';
2+
import { has, resolveModuleOrPackageSpecifier, safe } from '../../../utils/index.js';
33
import { handleJsDocType } from '../../../utils/jsdoc.js';
44

55
/**
@@ -95,7 +95,7 @@ export function handleJsDoc(doc, node) {
9595

9696

9797
/** @summary */
98-
if(tag?.tagName?.getText() === 'summary') {
98+
if(safe(() => tag?.tagName?.getText()) === 'summary') {
9999
doc.summary = tag.comment;
100100
}
101101

0 commit comments

Comments
 (0)