expressjs.com
expressjs.com copied to clipboard
expressjs
Use ESLint on JavaScript
- Updates the
testscript to run on JS files. - Whitelists
browserandjqueryas environments so their variables are seen as defined. - Resolves all the ESLint errors.
Deploy Preview for expressjscom-preview ready!
| Name | Link |
|---|---|
| Latest commit | b04cb65617dcc1cd95de2313fcdc7e7579373251 |
| Latest deploy log | https://app.netlify.com/projects/expressjscom-preview/deploys/684593b1af2a940008662663 |
| Deploy Preview | https://deploy-preview-1890--expressjscom-preview.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify project configuration.
![netlify[bot] avatar](https://avatars.githubusercontent.com/in/13473?v=4 "Published by a pub.dev verified publisher"){kind=small}
Hi @bdkopen, thank you for taking up this issue. Overall, the changes looks good! However, I would suggest moving the
eslintConfigsection from package.json into a separate file.
I moved the ESLint config into a new file. I responded to your other comments as well. Thanks for reviewing!
I fixed the test command so both Markdown and JS files are linted. Please let me know if there's anything else you'd like done in this PR.
at the very least, we still should have semicolons in code
Sure, I added a rule that overrides the standard default no semicolon rule.
oh... I didn't realize there would be so many missing semicolons. to unblock this, what we would ask is to change the configured rules such that the minimal amount of code change via --fix would apply. we don't want to blow up the git history right away. (maybe in the future though)
in other words, let's focus this PR so that linting is executing, but let's not change rules/files as part of it. or if that is not possible, which is understandable, this will need to wait till we resolve our future direction on linting
@ctcpip I dropped the commit enforcing semicolons. Without modifying the lint rules, the linting will remove semicolons in a few files since that is what the standard config defines.
This PR can be focused on requiring linting execution as long you're comfortable with enforcing the existing rules. If we don't want to enforce the existing rules, it seems like this PR will be blocked until the direction of linting is determined.
I think you could make it so that only JavaScript files have semicolons, and markdown files don’t have semicolons.
I think you could make it so that only JavaScript files have semicolons, and markdown files don’t have semicolons.
@bjohansebas that seems like a good intermediate solution. Updated!
It seems like there's not a clear direction with linting in the express project right now (https://github.com/expressjs/discussions/issues/327). I'll close this PR for now. If a linting standard is chosen, I'd be happy to help integrate it within the repos.