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

TS6133 Error if using typescript with noUnusedLocals and typescript preprocessor #322

Closed
OmarDarwish opened this issue Nov 1, 2017 · 11 comments

Comments

@OmarDarwish
Copy link

The functions declared in the output parser cause TS6133 errors when attempting to compile with typescript. This is because understandably, not all functions that are provided by builtins and generators are used in a user's grammar.

How to reproduce:

grammar.ne

@preprocessor typescript
@builtin "postprocessors.ne"

main -> foo
foo -> "foo"i
bash-3.2$ tsc -v
Version 2.6.1

bash-3.2$ nearleyc -v
2.11.0

bash-3.2$ nearleyc grammar.ne -o parser.ts

bash-3.2$ tsc --noUnusedLocals
parser.ts: error TS6133: 'id' is declared but never used.
parser.ts: error TS6133: 'nth' is declared but never used.
parser.ts: error TS6133: '$' is declared but never used.
@tjvr
Copy link
Collaborator

tjvr commented Nov 3, 2017

What is TS6133? Is this something TypeScript has turned on by default?

Any idea how we can fix or avoid this? :)

@OmarDarwish
Copy link
Author

OmarDarwish commented Nov 3, 2017

@tjvr TS6133 is thrown when you declare something but never use it. It is enforced if you use --noUnusedLocals, like in the example above. This option is off by default. It's a very useful feature of Typescript that helps with code cleanliness, so we have it on in most of our projects.

Pull #323 should fix this issue.

I wasn't sure if the proper etiquette for this project was to create a pull request or a pull request and an issue so I went with the more verbose option. ☺️

@glmdgrielson
Copy link

Isn't that an issue on your end? That seems like a good deal of complexity for something that is easily fixable by the user.

@OmarDarwish
Copy link
Author

@glmdgrielson Could you clarify please?

@glmdgrielson
Copy link

As far as I can tell, this issue has nothing to do with the Nearley module but instead is on your end. You said yourself that it's caused by your config, so why shouldn't you be able to fix it? Sorry if I'm being rude. I just don't see why it's such a big deal. 😕

@OmarDarwish
Copy link
Author

OmarDarwish commented Nov 27, 2017

Unfortunately, you can't configure tsc to ignore locals checking for certain files. Even if it did, I would have to add this to the generated parser every time. I would assume that Nearley, to the best of its ability, should be as helpful as it can be as a TypeScript module. I am certainly not the only user of Nearley who uses --noUnusedLocals.

It's an incredibly easy fix from the Nearley module side. Take a look at #323.

To accomplish the same thing as a user, they would have to either:

  • Reimplement postprocessors.ne by literally copying in the functions from master and adding // @ts-ignore to function declarations. This would mean that the copy would constantly have to maintained with Nearley releases.
  • Use builtin "postprocessors.ne" as is, but add dummy calls to unused functions when not necessary, which adds clutter and confusion.
  • Not use TypeScript's --noUnusedLocals

None of these user workarounds are ideal. A module side fix is much more simple and costs practically nothing to implement.

@tjvr
Copy link
Collaborator

tjvr commented Nov 28, 2017

// @ts-ignore seems fine to me. My worry is, will this hide other TS issues? Will think disable type-checking for that line, and is there any way that might cause problems?

Happy to merge if not 👍

@OmarDarwish
Copy link
Author

OmarDarwish commented Nov 28, 2017

I just tried. Type checking the invocation of a @ts-ignore function is still active 👌

@glmdgrielson
Copy link

The official website says:

A // @ts-ignore comment suppresses all errors that originate on the following line.

So... I'm not so sure. But if it works for you, then I guess that works!
On that note, that solves my problem handily.

@OmarDarwish
Copy link
Author

The following line with context to #323 is a function declaration, not invocation. Unused declaration is ignored, but the invocation is still scrutinized.

@tjvr tjvr closed this as completed in #323 Nov 29, 2017
@waseemminhas
Copy link

If you don't write first letter in capital for JS File Name /Class Name this issue can come up. I had this issue and resolved it by simply changing the file name to capital letter. I hope this might help.

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

No branches or pull requests

4 participants