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

Add Closure lint #93

Closed
wants to merge 3 commits into from
Closed

Add Closure lint #93

wants to merge 3 commits into from

Conversation

TimvdLippe
Copy link
Contributor

@TimvdLippe TimvdLippe commented Jun 10, 2018

Enable Closure linter to make sure Closure Compiler can work just fine on lit-element. I was unable to reproduce the issues @joncass was encountering.

Related to #80

@TimvdLippe TimvdLippe requested a review from sorvell as a code owner June 10, 2018 14:38
@joncass
Copy link

joncass commented Jun 11, 2018

Thanks for working on this @TimvdLippe. I was unable to get npm run lint to execute on the closure-lint branch. I get:

/dev/playground/lit-element$ npm run lint

> @polymer/[email protected] lint /dev/playground/lit-element
> tslint --project ./ && gulp closure

No valid rules have been specified for TypeScript files
[07:53:53] Using gulpfile ~/dev/playground/lit-element/gulpfile.js
[07:53:53] Starting 'closure'...
[07:53:53] 'closure' errored after 11 ms
[07:53:53] Error: File not found with singular glob: /dev/playground/lit-element/lit-element.js
    at Glob.<anonymous> (/dev/playground/lit-element/node_modules/glob-stream/index.js:41:11)
    at Object.onceWrapper (events.js:315:30)
    at emitOne (events.js:116:13)
    at Glob.emit (events.js:211:7)
    at Glob._finish (/dev/playground/lit-element/node_modules/glob-stream/node_modules/glob/glob.js:172:8)
    at done (/dev/playground/lit-element/node_modules/glob-stream/node_modules/glob/glob.js:159:12)
    at Glob._processSimple2 (/dev/playground/lit-element/node_modules/glob-stream/node_modules/glob/glob.js:652:12)
    at /dev/playground/lit-element/node_modules/glob-stream/node_modules/glob/glob.js:640:10
    at Glob._stat2 (/dev/playground/lit-element/node_modules/glob-stream/node_modules/glob/glob.js:736:12)
    at lstatcb_ (/dev/playground/lit-element/node_modules/glob-stream/node_modules/glob/glob.js:728:12)

Thoughts on what I might be doing wrong?

@TimvdLippe
Copy link
Contributor Author

TimvdLippe commented Jun 11, 2018 via email

@joncass
Copy link

joncass commented Jun 11, 2018

Got it, thanks. After running npm run build I can run npm run lint and it executes successfully.

However, when I change the language_out in the gulpfile to ES5 (which is what I'm using in my project) then I can see the error from #80

@TimvdLippe
Copy link
Contributor Author

Ah okay interesting. Will take a look 👍

@TimvdLippe
Copy link
Contributor Author

@joncass Per google/closure-compiler#1876 (comment) this appears to be a known limitation of the closure compiler. I am not sure what our options are in that case 😢

@joncass
Copy link

joncass commented Jun 18, 2018

Yeah, I found google/closure-compiler#2650 which indicated similar. However it's more about mixins being awkward to use, rather than impossible. If you look at the code snippet from the start of the thread it looks like a combination of what I originally proposed in #78 and duplicating a few declarations might be a workaround?

@TimvdLippe
Copy link
Contributor Author

I am no longer pursuing this PR.

@TimvdLippe TimvdLippe closed this Oct 5, 2019
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

Successfully merging this pull request may close these issues.

2 participants