Skip to content

Commit

Permalink
Overhaul linting + formatter approach
Browse files Browse the repository at this point in the history
The standard linter didn't play nice with Prettier, so that we had
standard undo certain formatting applied by Prettier. Thus we couldn't rely
on the Prettier extension in say VS Code to apply formatting right away
upon saving: an extra call in the CLI is necessary.

This change simplifies the approach: we're using Prettier for formatting
with an externalized config so that editors can pick it up, and eslint
for linting, and the two won't interfere.

This also allows us to add explicit steps for linting and formatting
checks in the CI pipeline.

We're also taking linting out of the "test" Grunt task. Unit tests and
linting should be two distinct things.
  • Loading branch information
carhartl committed Jun 22, 2023
1 parent 9fd191c commit 800142a
Show file tree
Hide file tree
Showing 17 changed files with 57 additions and 61 deletions.
2 changes: 1 addition & 1 deletion .eslintignore
@@ -1 +1 @@
examples/**/node_modules
dist
29 changes: 10 additions & 19 deletions .eslintrc.json
@@ -1,22 +1,13 @@
{
"extends": "standard",
"rules": {
"no-undef": "off",
"no-unused-vars": "off",
"no-var": "off",
"space-before-function-paren": "error"
"env": {
"browser": true,
"es2021": true
},
"plugins": ["html", "markdown"],
"overrides": [
{
"files": ["**/*.md"],
"processor": "markdown/markdown"
},
{
"files": ["**/*.md/*.javascript"],
"rules": {
"comma-dangle": ["error", "never"]
}
}
]
"extends": ["eslint:recommended", "prettier"],
"overrides": [],
"parserOptions": {
"ecmaVersion": "latest",
"sourceType": "module"
},
"rules": {}
}
10 changes: 6 additions & 4 deletions .github/workflows/ci.yml
Expand Up @@ -9,7 +9,6 @@ on:
- 'examples/**'
- '**.md'
- .gitignore
- .prettierignore
- .release-it.json
pull_request:
branches: [main]
Expand All @@ -19,13 +18,12 @@ on:
- 'examples/**'
- '**.md'
- .gitignore
- .prettierignore
- .release-it.json
schedule:
- cron: '0 0 1 * *' # Every month

jobs:
test:
build:
runs-on: ubuntu-latest
strategy:
matrix:
Expand All @@ -38,7 +36,11 @@ jobs:
node-version: ${{ matrix.node-version }}
- name: Install dependencies
run: npm i
- name: Unit tests
- name: Check formatting
run: npm run format:check
- name: Lint
run: npm run lint:check
- name: Run unit tests
run: npm test

e2e-test:
Expand Down
5 changes: 5 additions & 0 deletions .prettierrc.json
@@ -0,0 +1,5 @@
{
"semi": false,
"singleQuote": true,
"trailingComma": "none"
}
16 changes: 10 additions & 6 deletions Gruntfile.js
@@ -1,4 +1,5 @@
function encodingMiddleware (request, response, next) {
/* eslint-env node */
function encodingMiddleware(request, response, next) {
const URL = require('url').URL
const url = new URL(request.url, 'http://localhost')

Expand Down Expand Up @@ -90,10 +91,9 @@ const config = {
}
},
exec: {
format: 'npm run format',
lint: 'npm run lint',
rollup: 'npx rollup -c',
lint: 'npx standard',
format:
'npx prettier -l --write --single-quote --no-semi "**/*.{html,js,json,md,mjs,yml}" && npx eslint "**/*.{html,md}" --fix && npx standard --fix',
'browserstack-runner': 'node_modules/.bin/browserstack-runner --verbose'
}
}
Expand All @@ -107,7 +107,6 @@ module.exports = function (grunt) {
.forEach(grunt.loadNpmTasks)

grunt.registerTask('test', [
'exec:lint',
'exec:rollup',
'connect:build-qunit',
'qunit',
Expand All @@ -117,6 +116,11 @@ module.exports = function (grunt) {
'exec:rollup',
'exec:browserstack-runner'
])
grunt.registerTask('dev', ['exec:format', 'test', 'compare_size'])
grunt.registerTask('dev', [
'exec:format',
'exec:lint',
'test',
'compare_size'
])
grunt.registerTask('default', 'dev')
}
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -2,7 +2,7 @@
<img src="https://cloud.githubusercontent.com/assets/835857/14581711/ba623018-0436-11e6-8fce-d2ccd4d379c9.gif">
</p>

# JavaScript Cookie [![CI](https://github.com/js-cookie/js-cookie/actions/workflows/ci.yml/badge.svg)](https://github.com/js-cookie/js-cookie/actions/workflows/ci.yml) [![JavaScript Style Guide](https://img.shields.io/badge/code_style-standard-brightgreen.svg)](https://standardjs.com) [![Code Climate](https://codeclimate.com/github/js-cookie/js-cookie.svg)](https://codeclimate.com/github/js-cookie/js-cookie) [![npm](https://img.shields.io/github/package-json/v/js-cookie/js-cookie)](https://www.npmjs.com/package/js-cookie) [![size](https://img.shields.io/bundlephobia/minzip/js-cookie/3)](https://www.npmjs.com/package/js-cookie) [![jsDelivr Hits](https://data.jsdelivr.com/v1/package/npm/js-cookie/badge?style=rounded)](https://www.jsdelivr.com/package/npm/js-cookie)
# JavaScript Cookie [![CI](https://github.com/js-cookie/js-cookie/actions/workflows/ci.yml/badge.svg)](https://github.com/js-cookie/js-cookie/actions/workflows/ci.yml) [![Code Climate](https://codeclimate.com/github/js-cookie/js-cookie.svg)](https://codeclimate.com/github/js-cookie/js-cookie) [![npm](https://img.shields.io/github/package-json/v/js-cookie/js-cookie)](https://www.npmjs.com/package/js-cookie) [![size](https://img.shields.io/bundlephobia/minzip/js-cookie/3)](https://www.npmjs.com/package/js-cookie) [![jsDelivr Hits](https://data.jsdelivr.com/v1/package/npm/js-cookie/badge?style=rounded)](https://www.jsdelivr.com/package/npm/js-cookie)

A simple, lightweight JavaScript API for handling cookies

Expand Down
1 change: 1 addition & 0 deletions examples/webpack/server.js
@@ -1,3 +1,4 @@
/* eslint-env node */
const nodeStatic = require('node-static')
const file = new nodeStatic.Server('./dist')
const port = 8080
Expand Down
1 change: 1 addition & 0 deletions index.js
@@ -1 +1,2 @@
/* eslint-env node */
module.exports = require('./dist/js.cookie')
10 changes: 6 additions & 4 deletions package.json
Expand Up @@ -28,7 +28,10 @@
],
"scripts": {
"test": "grunt test",
"format": "grunt exec:format",
"format": "prettier --list-different --write .",
"format:check": "prettier --list-different .",
"lint": "eslint --ext .js,.mjs --fix .",
"lint:check": "eslint --ext .js,.mjs .",
"dist": "rm -rf dist/* && rollup -c",
"release": "release-it"
},
Expand All @@ -46,7 +49,7 @@
"@rollup/plugin-terser": "^0.4.0",
"browserstack-runner": "github:browserstack/browserstack-runner#1e85e559951bdf97ffe2a7c744ee67ca83589fde",
"eslint": "^8.43.0",
"eslint-config-standard": "^17.1.0",
"eslint-config-prettier": "^8.8.0",
"eslint-plugin-html": "^7.0.0",
"eslint-plugin-markdown": "^3.0.0",
"grunt": "^1.0.4",
Expand All @@ -62,8 +65,7 @@
"release-it": "^15.0.0",
"rollup": "^3.17.2",
"rollup-plugin-filesize": "^10.0.0",
"rollup-plugin-license": "^3.0.0",
"standard": "^17.0.0"
"rollup-plugin-license": "^3.0.0"
},
"engines": {
"node": ">=16"
Expand Down
12 changes: 6 additions & 6 deletions src/api.mjs
@@ -1,9 +1,8 @@
/* eslint-disable no-var */
import assign from './assign.mjs'
import defaultConverter from './converter.mjs'

function init (converter, defaultAttributes) {
function set (name, value, attributes) {
function init(converter, defaultAttributes) {
function set(name, value, attributes) {
if (typeof document === 'undefined') {
return
}
Expand Down Expand Up @@ -47,7 +46,7 @@ function init (converter, defaultAttributes) {
name + '=' + converter.write(value, name) + stringifiedAttributes)
}

function get (name) {
function get(name) {
if (typeof document === 'undefined' || (arguments.length && !name)) {
return
}
Expand All @@ -67,7 +66,9 @@ function init (converter, defaultAttributes) {
if (name === found) {
break
}
} catch (e) {}
} catch (e) {
// Do nothing...
}
}

return name ? jar[name] : jar
Expand Down Expand Up @@ -101,4 +102,3 @@ function init (converter, defaultAttributes) {
}

export default init(defaultConverter, { path: '/' })
/* eslint-enable no-var */
2 changes: 0 additions & 2 deletions src/assign.mjs
@@ -1,4 +1,3 @@
/* eslint-disable no-var */
export default function (target) {
for (var i = 1; i < arguments.length; i++) {
var source = arguments[i]
Expand All @@ -8,4 +7,3 @@ export default function (target) {
}
return target
}
/* eslint-enable no-var */
2 changes: 0 additions & 2 deletions src/converter.mjs
@@ -1,4 +1,3 @@
/* eslint-disable no-var */
export default {
read: function (value) {
if (value[0] === '"') {
Expand All @@ -13,4 +12,3 @@ export default {
)
}
}
/* eslint-enable no-var */
3 changes: 0 additions & 3 deletions test/encoding.js
@@ -1,5 +1,4 @@
/* global QUnit, lifecycle, using */
/* eslint-disable no-var */

QUnit.module('cookie-value', lifecycle)

Expand Down Expand Up @@ -1169,5 +1168,3 @@ QUnit.test('cookie-name - 4 bytes characters', function (assert) {
)
})
})

/* eslint-enable no-var */
2 changes: 1 addition & 1 deletion test/missing_semicolon.html
Expand Up @@ -6,7 +6,7 @@
<script>
window.__ok = false
;(function () {
function loadFileSync (path) {
function loadFileSync(path) {
var xhr = new window.XMLHttpRequest()
xhr.open('GET', path, false)
xhr.send(null)
Expand Down
1 change: 1 addition & 0 deletions test/node.js
@@ -1,3 +1,4 @@
/* eslint-env node */
exports.node = {
shouldLoadApi: function (test) {
test.expect(1)
Expand Down
9 changes: 3 additions & 6 deletions test/tests.js
@@ -1,5 +1,4 @@
/* global Cookies, QUnit, lifecycle, quoted */
/* eslint-disable no-var */

QUnit.module('setup', lifecycle)

Expand All @@ -18,7 +17,7 @@ QUnit.test('api instance creation', function (assert) {
)

api = Cookies.withConverter({
write: function (value, name) {
write: function (value) {
return value.toUpperCase()
}
}).withAttributes({ path: '/foo' })
Expand All @@ -28,7 +27,7 @@ QUnit.test('api instance creation', function (assert) {
)

api = Cookies.withAttributes({ path: '/foo' }).withConverter({
write: function (value, name) {
write: function (value) {
return value.toUpperCase()
}
})
Expand Down Expand Up @@ -252,7 +251,7 @@ QUnit.test('String primitive', function (assert) {
})

QUnit.test('String object', function (assert) {
/* eslint-disable no-new-wrappers */
// eslint-disable-next-line no-new-wrappers
Cookies.set('c', new String('v'))
assert.strictEqual(Cookies.get('c'), 'v', 'should write value')
})
Expand Down Expand Up @@ -605,5 +604,3 @@ QUnit.test('do not conflict with existent globals', function (assert) {
)
window.Cookies = Cookies
})

/* eslint-enable no-var */
11 changes: 5 additions & 6 deletions test/utils.js
@@ -1,5 +1,4 @@
/* global Cookies */
/* eslint-disable no-var */

;(function () {
window.lifecycle = {
Expand All @@ -18,7 +17,7 @@
}

window.using = function (assert) {
function getQuery (key) {
function getQuery(key) {
var queries = window.location.href.split('?')[1]
if (!queries) {
return
Expand All @@ -30,7 +29,7 @@
return decodeURIComponent(result)
}
}
function setCookie (name, value) {
function setCookie(name, value) {
return {
then: function (callback) {
var iframe = document.getElementById('request_target')
Expand All @@ -54,7 +53,9 @@
)
callback(result.value, iframeDocument.cookie)
done()
} catch (e) {}
} catch (e) {
// Do nothing...
}
})
iframe.src = requestURL
}
Expand All @@ -70,5 +71,3 @@
return '"' + input + '"'
}
})()

/* eslint-enable no-var */

0 comments on commit 800142a

Please sign in to comment.