tools
tools copied to clipboard
☂️ Parser bugs and problems
Description
Not everything on this list is a bug. They are bullet points that we need to address. We can choose to keep it like it is today.
Tasks
- [ ] Top level return
- [ ] export name from 'a'
- [x] export { \n default as a } from 'a'
- [ ] multiple "export default"
- [ ] const arguments/package and others invalid bindings
- [ ] export anonymous function on .d.ts
- [x] Not all string are directives
Top Level Return
Some node.js scripts are using "top level return".
https://github.com/ant-design/ant-design/blob/master/scripts/check-version-md.js https://stackoverflow.com/questions/28955047/why-does-a-module-level-return-statement-work-in-node-js
https://play.rome.tools/?lineWidth=80&indentStyle=tab&indentWidth=2&typescript=true&jsx=true#cmV0dXJuOw==
Examples:
https://github.com/ant-design/ant-design/blob/master/scripts/check-version-md.js#L60
https://github.com/marmelab/react-admin/blob/master/cypress/start.js#L4
Export from
We don't support "export default from" and have a problem with "export { \n default ..." (mind the new line):
https://play.rome.tools/?lineWidth=80&indentStyle=tab&indentWidth=2&typescript=true&jsx=true#ZXhwb3J0IGRlZmF1bHQgZnJvbSAnYSc7CmV4cG9ydCB7IGRlZmF1bHQgYXMgQSB9IGZyb20gJ2EnOwpleHBvcnQgeyAKICAgIGRlZmF1bHQgYXMgQSB9IGZyb20gJ2EnOw==
babel seems to like "export default from 'a';"; ts on the other hand does not.
https://babeljs.io/repl#?browsers=defaults%2C%20not%20ie%2011%2C%20not%20ie_mob%2011&build=&builtIns=false&corejs=3.21&spec=false&loose=false&code_lz=KYDwDg9gTgLgBAE2AMwIYFcA29lQgWzgHJUiBuIA&debug=false&forceAllTransforms=false&shippedProposals=false&circleciRepo=&evaluate=false&fileSize=false&timeTravel=false&sourceType=module&lineWrap=true&presets=env%2Creact%2Cstage-2&prettier=false&targets=&version=7.17.8&externalPlugins=%40babel%2Fplugin-proposal-export-default-from%407.16.7&assumptions=%7B%7D
https://ts-ast-viewer.com/#code/KYDwDg9gTgLgBAE2AMwIYFcA29lQgWzgHJUiBuIA
Another issue is
export getComponentGroupInfo from './getComponentGroupInfo'
https://play.rome.tools/?lineWidth=80&indentStyle=tab&indentWidth=2&typescript=true&jsx=true#ZXhwb3J0IGdldENvbXBvbmVudEdyb3VwSW5mbyBmcm9tICcuL2dldENvbXBvbmVudEdyb3VwSW5mbyc=
JSX
For some reason we can parse "" but not "="
https://play.rome.tools/?lineWidth=80&indentStyle=tab&indentWidth=2&typescript=true&jsx=true#PHNwYW4+PC9zcGFuPgo8c3Bhbj49PC9zcGFuPg==
The problem is that ">=" is lexed as a token and breaks the JSX parsing.
0: [email protected] "<" [] []
1: [email protected]
0: [email protected] "span" [] []
2: [email protected]
0: [email protected]
0: [email protected] ">=" [] []
Multiple Export default
For example, react-hook-form uses multiple "export default function".
https://github.com/react-hook-form/react-hook-form/blob/master/examples/V7/nestedFields.tsx
This is unanimously frowned upon across all parsers with errors; but running on Stackblitz, for example, actually works. Although the result is not intuitive at all: https://stackblitz.com/edit/js-mnyyat
Const arguments/package and others invalid bindings
This is a little bit silly, but ts and node actually support variables named "arguments" and we do not.
https://play.rome.tools/?lineWidth=80&indentStyle=tab&indentWidth=2&typescript=true&jsx=true#Y29uc3QgYXJndW1lbnRzID0gMTs=
https://ts-ast-viewer.com/#code/MYewdgzgLgBAhgJwOYFcC2BTMUIwLwwCMA3EA
Example of use: https://github.com/reduxjs/redux/blob/master/scripts/mangleErrors.js#L74
https://github.com/marmelab/react-admin/blob/master/jest.config.js#L4
Export anonymous function on .d.ts
On .d.ts, the code below is not working.
export default function() {}
error[SyntaxError]: expected an identifier, an array pattern, or an object pattern but instead found '('
┌─ parser_smoke_test:1:24
│
1 │ export default function() {}
│ ^ Expected an identifier, an array pattern, or an object pattern here
error[SyntaxError]: A 'declare' function cannot have a function body
┌─ parser_smoke_test:1:27
│
1 │ export default function() {}
│ ^^ remove this body
Not all string are directives
We treat lines purely with strings at the beginning of methods as directives. This is not always the case:
function a() {
'a'
.something()
}
https://play.rome.tools/?lineWidth=80&indentStyle=tab&indentWidth=2&typescript=true&jsx=true#ZnVuY3Rpb24gYSgpIHsKICAnYScKICAuc29tZXRoaW5nKCkKfQ==
Real case: https://github.com/OnsenUI/OnsenUI/blob/master/onsenui/esm/ons/internal/swipe-reveal.js#L28
Thanks @xunilrj for assembling this list. I've started comparing the issues with the spec
- top level return: Not spec compliment. Neither
ScriptBodynorModuleBodyallow for top level return spec. export default from. Not yet standardized in ECMA 2022 spec. Currently a stage 1 proposalexport name from ...: Not standardized spec. Also part of the export default proposal- JSX: That's funny. I'll have a look
- Multiple default exports: Incorrect to my understanding. Duplicate export names isn't permitted according to the spec
arguments: Forbiddingargumentsis correct in strict mode spec- Directives: Yes, our parsing is incorrect. It's any sequence of expressions spec
More problems found in the wild:
- [ ] Optional binding on .d.ts
- [ ] Export anonymous function in .d.ts
- [ ] Initializers in ambient contexts
- [x] Strange unicodes character inside regex
Optional binding on .d.ts
declare namespace A {
class ErrorsTextOptions {
separator;
dataVar;
}
export class Ajv {
errorsText(errors?: string[] | null | undefined, { separator, dataVar }?: ErrorsTextOptions): string;
}
}
error[SyntaxError]: A binding pattern parameter cannot be optional in an implementation signature.
┌─ main.js:7:52
│
7 │ errorsText(errors?: string[] | null | undefined, { separator, dataVar }?: ErrorsTextOptions): string;
│ ^^^^^^^^^^^^^^^^^^^^^^
https://play.rome.tools/?lineWidth=80&indentStyle=tab&indentWidth=2&typescript=true&jsx=false#ZGVjbGFyZSBuYW1lc3BhY2UgQSB7CmNsYXNzIEVycm9yc1RleHRPcHRpb25zIHsKICBzZXBhcmF0b3I7CiAgZGF0YVZhcjsKfQpleHBvcnQgY2xhc3MgQWp2IHsKICBlcnJvcnNUZXh0KGVycm9ycz86IHN0cmluZ1tdIHwgbnVsbCB8IHVuZGVmaW5lZCwgeyBzZXBhcmF0b3IsIGRhdGFWYXIgfT86IEVycm9yc1RleHRPcHRpb25zKTogc3RyaW5nOwp9Cn0=
ts has no problem with this:
https://ts-ast-viewer.com/#code/CYUwxgNghgTiAEA7KBbEBnADlMCCC8A3gFCRTrrwCiMMA9jOgCogAeALgPKbsCWdiSiXjx0IbDCjsGAbmIjgUqADVYcgL7E2mBu3hkK8PACsAbkXnwQtBszbsAFNfqMA-AC5R7GL0QBzAG0AXXgAHyQAVwgIMPgIxFAAM18QYAAaIlFxWCkGDMV2FVh4dQ9qG0YWDm4+AXQASk90b18-DWJ1IA
see:
- https://unpkg.com/[email protected]/dist/core.d.ts
- https://unpkg.com/[email protected]/lib/readFromStore.d.ts
Export anonymous function in .d.ts
export default function (objA: any, objB: any): boolean;
│
1 │ export default function (objA: any, objB: any): boolean;
│ ^ Expected an identifier, an array pattern, or an object pattern here
see: https://unpkg.com/[email protected]/es/utils/shallowEqual.d.ts
Initializers in ambient contexts
I think this is valid since ts version 2.1.
declare namespace A {
export class ViewHelper {
readonly isRootView = false;
}
}
error[SyntaxError]: Initializers are not allowed in ambient contexts.
┌─ main.js:3:29
│
3 │ readonly isRootView = false;
│ ^^^^^^^
https://play.rome.tools/?lineWidth=80&indentStyle=tab&indentWidth=2&typescript=true&jsx=false#ZGVjbGFyZSBuYW1lc3BhY2UgQSB7CiAgICBleHBvcnQgY2xhc3MgVmlld0hlbHBlciB7CiAgICAgICAgcmVhZG9ubHkgaXNSb290VmlldyA9IGZhbHNlOwogICAgfQp9
https://ts-ast-viewer.com/#code/CYUwxgNghgTiAEA7KBbEBnADlMCCC8A3gFDxnwgAemA9jAC7yRTrrwBqAliAO4ASICJhAwipchLhRgNRBACe8TugBKNGvS694AXngAzKBHQgA3OLIBfYpaA
Strange unicode characters inside regex
Our regex parser is getting lost with some "strange unicode" characters.
var matchMonthPatterns = {
// eslint-disable-next-line no-misleading-character-class
narrow: /^[يفمئامئئاسۆند]/i
}
error[SyntaxError]: expected `,` but instead found ``
┌─ parser_smoke_test:3:27
│
3 │ narrow: /^[يفمئامئئاسۆند]/i
│ unexpected
error: unterminated regex literal
┌─ parser_smoke_test:3:27
│
3 │ narrow: /^[يفمئامئئاسۆند]/i
│ - ...but the line ends here
│ │
│ a regex literal starts there...
error: Unexpected token ``
┌─ parser_smoke_test:3:27
│
3 │ narrow: /^[يفمئامئئاسۆند]/i
│
This also bugs the playground.
https://ts-ast-viewer.com/#code/G4QwTgBAtiAuDGALAsgewHa0QBTrApmOgM4QC8EA3gFAR310D0jE+xANgJaYC0AJp2IgARu3w90+AB6weXSRHSoeUQWJAD0Acx5JwIeATC72IYsVoN66cGFQB3AFwRGAPQDagKTBAgmCBRMEBkYIDkYAGAsARBgMxggGNggGJggPRgALqMnJZ0AL5AA
see: https://unpkg.com/[email protected]/locale/ug/_lib/match/index.js
I found a couple of bugs. We fail to parse two files inside the TypeScript repository:
https://github.com/microsoft/TypeScript/blob/main/src/lib/webworker.generated.d.ts
error[SyntaxError]: Illegal use of `arguments` as an identifier in strict mode
┌─ main.js:5189:61
│
5189 │ setInterval(handler: TimerHandler, timeout?: number, ...arguments: any[]): number;
│ ^^^^^^^^^
error[SyntaxError]: Illegal use of `arguments` as an identifier in strict mode
┌─ main.js:5190:60
│
5190 │ setTimeout(handler: TimerHandler, timeout?: number, ...arguments: any[]): number;
│ ^^^^^^^^^
The other file is: https://github.com/microsoft/TypeScript/blob/main/src/lib/dom.generated.d.ts for the same reason
I found a couple of bugs. We fail to parse two files inside the TypeScript repository:
https://github.com/microsoft/TypeScript/blob/main/src/lib/webworker.generated.d.ts
error[SyntaxError]: Illegal use of `arguments` as an identifier in strict mode ┌─ main.js:5189:61 │ 5189 │ setInterval(handler: TimerHandler, timeout?: number, ...arguments: any[]): number; │ ^^^^^^^^^ error[SyntaxError]: Illegal use of `arguments` as an identifier in strict mode ┌─ main.js:5190:60 │ 5190 │ setTimeout(handler: TimerHandler, timeout?: number, ...arguments: any[]): number; │ ^^^^^^^^^The other file is: https://github.com/microsoft/TypeScript/blob/main/src/lib/dom.generated.d.ts for the same reason
I guess the issue here is that we parse js files as modules by default. That means they get parsed in strict mode, in which arguments is disallowed. Fixing this would likely require that our parser correctly infers whatever a file is a module or a script depending if it uses any ES module syntax or not, but that's not as straightforward.
This is a .d.ts file though.
This is a
.d.tsfile though.
Here it's the same. We, by default, assume it's a module and not a script.
@xunilrj this is probably related to https://github.com/rome/tools/issues/2960 ? Can we track everything in one single issue?