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

Use eslint instead of jshint #604

Merged
merged 12 commits into from
Oct 6, 2022
Merged

Use eslint instead of jshint #604

merged 12 commits into from
Oct 6, 2022

Conversation

craigbeck
Copy link
Contributor

  • Add eslintrc rules converted from jshintrc via polyjuice
  • Allow == checks with null as this effective for null or undefined checks
  • Split lint and test actions into individual scripts
  • Add checks script combining both lint and test
  • Code processed with eslint --fix for simple issues (formatting, commas etc)
  • Change expressions to conditionals or use default parameters where appropriate

lib/textDiff.js Outdated
@@ -2,20 +2,18 @@ exports.onStringInsert = onStringInsert;
exports.onStringRemove = onStringRemove;
exports.onTextInput = onTextInput;

function onStringInsert(el, previous, index, text) {
function onStringInsert(el, previous = '', index, text) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not use a default param here, to keep ES5 compatibility for now, and to make for easier future TS conversion.

if (!previous) {
  previous = '';
}

lib/textDiff.js Outdated
var newText = previous.slice(0, index) + text + previous.slice(index);
replaceText(el, newText, transformCursor);
}

function onStringRemove(el, previous, index, howMany) {
function onStringRemove(el, previous = '', index, howMany) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

@@ -257,7 +257,9 @@ EventModel.prototype._removeItemContext = function(context) {

EventModel.prototype._addBinding = function(binding) {
var bindings = this.bindings || (this.bindings = new BindingsMap());
binding.eventModels || (binding.eventModels = new EventModelsMap());
if (bindings.eventModel == null) {
binding.eventModels = new EventModelsMap()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add semicolon at end

lib/files.js Outdated
@@ -54,7 +54,9 @@ function loadViewsSync(app, sourceFilename, namespace) {
}

function loadStylesSync(app, sourceFilename, options) {
options || (options = {compress: util.isProduction});
if (options == null) {
options = { compress: util.isProduction }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Semicolon at end

@craigbeck craigbeck merged commit d2cfca5 into derby-2 Oct 6, 2022
@@ -257,7 +257,9 @@ EventModel.prototype._removeItemContext = function(context) {

EventModel.prototype._addBinding = function(binding) {
var bindings = this.bindings || (this.bindings = new BindingsMap());
binding.eventModels || (binding.eventModels = new EventModelsMap());
if (bindings.eventModel == null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should actually be:

if (binding.eventModels == null) {

Issue was caught by internal tests when testing derby-2 branch. I'll try and write a more minimal test in the Derby tests.

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