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 eslint env/config for ember-electron projects #357

Open
jacobq opened this issue Jul 17, 2018 · 8 comments
Open

Add eslint env/config for ember-electron projects #357

jacobq opened this issue Jul 17, 2018 · 8 comments

Comments

@jacobq
Copy link
Collaborator

jacobq commented Jul 17, 2018

Would it make sense to make an eslint plugin that would get added during ember install ember-electron?
Mainly, it'd be nice not to have to manually fix the error 'requireNode' is not defined no-undef entries, but perhaps there are other things to note.

@jacobq
Copy link
Collaborator Author

jacobq commented Jul 17, 2018

Is there a reason we're running standard to check mocha test style vs. doing this as part of an eslint plugin like eslint-plugin-mocha?
cc @pichfl
cc @felixrieseberg

@jacobq
Copy link
Collaborator Author

jacobq commented Jul 27, 2018

Rather than an eslint plugin, perhaps we could just add some rules in the override section. This change might fit together nicely with the proposal of removing the .eslintrc in the blueprint.

@jacobq
Copy link
Collaborator Author

jacobq commented Sep 18, 2018

Also, it looks like the electron-out folder is not necessarily ignored by eslint (runs on the project root), so we may want to consider adding a line to .eslintignore to exclude it.

@bendemboski
Copy link
Member

Much of this is fixed in v3, however we kinda punted on linting. It does lint out-of-box, but we added electron-app to .eslintignore and just don't lint that project.

At some point I do think it would make sense to revisit, and figure out a good default linting config for the electron-app project. On the other hand, electron-forge doesn't have any linting config, and I was unable to find much in the way of established practices for Electron projects...so not sure if we should just let people roll their own until/unless the Electron community starts putting together some linting practices.

@RobbieTheWagner
Copy link
Member

I would argue for adding eslint-plugin-node, like ember does by default. Thoughts?

@jacobq
Copy link
Collaborator Author

jacobq commented Nov 26, 2019

Before suggesting solutions, perhaps we could try to make a list of the main goals / pain-points we'd like linting to help with. Things that come to mind immediately are processNode and requireNode.

If there isn't already a set of linting rules for electron, we might want consider creating a separate package for that and configuring ember-electron projects to use them for JS files in electron-app. In particular, I'm thinking of warning about use of remote after reading "Electron’s ‘remote’ module considered harmful".

@bendemboski
Copy link
Member

TL;DR: I think we should write up an FAQ on linting, and leave the blueprint as-is.

My opinion here is that we should keep ember-electron focused on its core job, which is to integrate Ember with Electron. I think this involves three goals:

  1. Instrument Ember framework/runtime as needed to support integration (e.g. the require/requireNode stuff to allow node integration to coexist with Ember's loader.js, or the electron-protocol-serve stuff to allow Ember's routing to work when loading from the filesystem rather than http: URLs
  2. Allow Ember tooling (ember-cli) to coexist with Electron tooling (electron-forge) to create an integrated development environment (e.g. ember electron:* commands)
  3. Provide a good "quickstart" experience and good documentation to support Ember developers to learn about Electron using their experience with Ember as a starting point

Feel free to suggest other goals I'm maybe not thinking of.

If someone were to make a feature request that has nothing to do with the above, but is entirely in the domain of Electron/electron-forge (e.g. requesting a way to determine from main process code if we're running is a development environment or in a packaged app), I would argue for at most adding info to an FAQ or something, but not trying to fix things that don't have anything to do with the Ember/Electron integration. This is a similar philosophy to what Ember has been doing recently in removing stuff like Ember.Evented that doesn't have anything to do with Ember's core value proposition, or the long-standing philosophy of putting things in addons when it's possible and there isn't a really really good argument for putting it in the core.

linting is kind of a tricky one, and we can definitely argue about it -- Ember provides out-of-box linting, so should we just be making sure it doesn't break the Electron project (which is what we're doing now by simply ignoring it in .eslintrc.js), or should we call full linting an
"integrated development environment" feature and try to actually lint the Electron project?

The linting story in the Electron ecosystem seems pretty un-developed. Try a google search and you'll mostly find results geared towards contributing to the Electron project itself. electron-forge has a lint command, but it's just a thin wrapper around yarn-or-npm lint, and the default electron-forge template configures the lint script to be a no-op.

So I'm nervous about trying to forge (heh) our own path linting Electron projects, because I don't think we have enough experience with this new project layout to have a good sense of the right direction to go, and we have no guidance from Electron/electron-forge, nor can we predict what direction they will go in to figure out how that would fit into our ember-electron integrated developer environment. So I think we're best off not adding this to our "API surface area" until we know more, and maybe our efforts would best be spent building up some tooling focused on Electron apps (e.g. eslint-plugin-electron or something) to help move the Electron ecosystem forward into a world with a good linting story.

If y'all agree, I'd be happy to write up an FAQ on linting, with my config as an example (I do have full linting working in my ember-electron app) so we can point people there and then start collecting some usage results.

@RobbieTheWagner
Copy link
Member

@bendemboski I think documenting would be an adequate fix here. Linting is very personal to most folks anyway, so forcibly editing their config may not be what we want anyway.

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

3 participants