Skip to content
This repository was archived by the owner on Nov 8, 2024. It is now read-only.

Fix eslint warnings #141

Merged
merged 1 commit into from
Feb 5, 2019
Merged

Fix eslint warnings #141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/fury-adapter-oas3-parser/test/unit/chai.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* eslint-disable no-unused-expressions */
/* eslint-disable no-unused-vars */
/* eslint-disable no-underscore-dangle */

Expand Down
1 change: 0 additions & 1 deletion packages/fury/lib/fury.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ class Fury {
/*
* Load serialized refract elements into Javascript objects.
*/
// eslint-disable-next-line class-methods-use-this
load(elements) {
return this.minim.fromRefract(elements);
}
Expand Down
3 changes: 1 addition & 2 deletions packages/fury/test/fury-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* eslint-disable global-require */
const { assert } = require('chai');
const fury = require('../lib/fury');

Expand Down Expand Up @@ -268,7 +267,7 @@ const refractedApi = {

describe('Nodes.js require', () => {
it('should work without needing to use `.default`', () => {
assert(require('../lib/fury').parse);
assert(fury.parse);
Copy link
Contributor

Choose a reason for hiding this comment

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

This change sounds like it makes the test not do what it is described to do.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's testing whether .parse works without .default. The fury is const fury = require('../lib/fury');, so it's equivalent.

Copy link
Contributor

Choose a reason for hiding this comment

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

@honzajavorek Ah, thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, although IMO I dislike the global version of Fury that is returned here. Personally I would like to consider dropping this functionality so that require('fury') returns the Fury class which the consumer must instansiate their own version and add the adapters. I don't like singleton state that is currently permitted.

Along with #32 we would be renaming the libraries and Fury would become API Elements Core. In that case, I think it would make sense to make "Fury" itself a Namespace and thus perhaps in the future we would have something like:

const { Namespace } = require('@apielements/core');
const APIBlueprintParser = require('@apielements/apib-parser');

const namespace = new Namespace();
namespace.use(APIBlueprintParser);

const parseResult = await namespace.parse({ source });

As the biggest consumers of Fury (Dredd/Docs) I would love to hear some feedback regarding these potential changes. I was thinking that Namespace here is actually a subclass of minim namespace (if Minim continues to exist standalone). It is a namespace with additional methods detect, validate, parse and compose/serialise.

I have also been pondering the idea of a complete API Elements JS package which incorporates all the adapters for all formats so you don't have to worry about individual support.

const { Namespace } = require('apielements');

const namespace = new Namespace();

const parseResult = await namespace.parse({ source });

In your cases it may make sense to depend on apielements whereas the separate core, and adapter packages are provided for consumers who wish to mix and match adapters. Although you both are not using the serialisers and this may add additional dependency bloat so it may not be desired.

});
});

Expand Down
7 changes: 3 additions & 4 deletions packages/fury/test/validate-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* eslint-disable no-empty-pattern */
const { expect } = require('chai');
const { Fury } = require('../lib/fury');

Expand All @@ -13,7 +12,7 @@ describe('Validation', () => {
name: 'passthrough',
mediaTypes: ['text/vnd.passthrough'],
detect: () => shouldDetect,
validate: ({}, done) => done(null, result),
validate: (options, done) => done(null, result),
});

beforeEach(() => {
Expand Down Expand Up @@ -118,8 +117,8 @@ describe('Validation', () => {
fury.use({
name: 'passthrough',
mediaTypes: ['text/vnd.passthrough'],
detect: ({}) => true,
parse: ({}, done) => done(null, result),
detect: () => true,
parse: (options, done) => done(null, result),
});

before(() => {
Expand Down
1 change: 0 additions & 1 deletion packages/minim-parse-result/test/parse-result-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,6 @@ describe('Parse result namespace', () => {
});

it('should contain a sourceMap attribute with one item', () => {
// eslint-disable-next-line no-unused-expressions
expect(sourceMaps).to.exist;
expect(sourceMaps).to.have.length(1);
});
Expand Down