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

chore(prettier): [RO-26549] Added Prettier #82

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 6 additions & 4 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
root = true

[*]
indent_style = space
indent_size = 2
end_of_line = lf
charset = utf-8
trim_trailing_whitespace = true
end_of_line = lf
indent_size = 2
indent_style = space
insert_final_newline = true
max_line_length = 120
quote_type = single
trim_trailing_whitespace = true

[node_modules/**.js]
codepaint = false
33 changes: 0 additions & 33 deletions .eslintrc

This file was deleted.

10 changes: 10 additions & 0 deletions .eslintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"env": {
"node": true,
"es2021": true
},
"plugins": ["node"],
"parserOptions": {
"ecmaVersion": "latest"
}
}
51 changes: 22 additions & 29 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

Common node modules and express middleware that are designed to be the boilerplate of a node web app.

[![NPM Version][npm-version-image]][npm-url]
[![NPM Install Size][npm-install-size-image]][npm-install-size-url]
[![NPM Downloads][npm-downloads-image]][npm-downloads-url]
[![NPM Version][npm-version-image]][npm-url]
[![NPM Install Size][npm-install-size-image]][npm-install-size-url]
[![NPM Downloads][npm-downloads-image]][npm-downloads-url]

# About the Spur Framework

Expand All @@ -15,7 +15,7 @@ The Spur Framework is a collection of commonly used Node.JS libraries used to cr
# Topics

- [Quick start](#quick-start)
- [Usage](#usage)
- [Usage](#usage)
- [Available dependencies in injector](#available-dependencies-in-injector)
- [Contributing](#contributing)
- [License](#license)
Expand All @@ -25,11 +25,13 @@ The Spur Framework is a collection of commonly used Node.JS libraries used to cr
## Installing

`Dependencies:`

```shell
$ npm install --save spur-ioc spur-common spur-config
```

`Module:`

```shell
$ npm install --save spur-web
```
Expand Down Expand Up @@ -57,15 +59,11 @@ module.exports = function () {
// Register configuration
registerConfig(ioc, path.join(__dirname, './config'));


ioc.merge(spurCommon());
ioc.merge(spurWeb());

// register folders in your project to be auto-injected
ioc.registerFolders(__dirname, [
'controllers/',
'runtime/'
]);
ioc.registerFolders(__dirname, ['controllers/', 'runtime/']);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not so much for this one, but in harness, can we please not rewrite the examples in markdown? This is not code so it's optimized for reading in the doc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was formatted by Prettier. .md files could be ignored but then you lose some of the benefits like table formatting, and consistency across .md files in the repo.

I'm not convinced this is an issue. Depending the formatting setup of the target repo that you're copy/pasting into, it would get reformatted anyway right? i.e. If this was left unchanged and you pasted the example code into a repo using Prettier, it would be format like this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a public repo, it's for readability since we don't impose our formatting on anyone.


return ioc;
};
Expand All @@ -76,7 +74,6 @@ module.exports = function () {
```javascript
module.exports = function (BaseWebServer, path) {
class WebServer extends BaseWebServer {

// Add additional changes to the middleware by overriding the method
registerDefaultMiddleware() {
super.registerDefaultMiddleware();
Expand All @@ -95,7 +92,6 @@ Files ending in `*Controller.js` are auto registered as controllers.
```javascript
module.exports = function (BaseController) {
class HelloController extends BaseController {

configure(app) {
super.configure(app);

Expand All @@ -110,7 +106,6 @@ module.exports = function (BaseController) {
getHello(req, res) {
res.send('hello');
}

}

return new HelloController();
Expand All @@ -130,8 +125,7 @@ injector().inject(function (UncaughtHandler, WebServer, Logger, config, configLo
Logger.info(`PORT: ${config.Port}`);
Logger.info(`CONFIG: ${configLoader.configName}`);

WebServer.start()
.then(() => {
WebServer.start().then(() => {
// Execute other logic after the server has started
});
});
Expand All @@ -152,7 +146,7 @@ To see the latest list of the default dependencies that are injected, check out
List of external dependencies used and exposed by spur-web. They can be found at npmjs.org using their original names.

| Name | Original Module Name |
| :---- | :---- |
| :----------------- | :--------------------------------------------------------------- |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did prettier add these? :---- is a markdown shortcut to make it dynamic and not have to keep track of the length as you write. Can you please revert?

Copy link
Contributor Author

@otrreeves otrreeves Apr 4, 2024

Choose a reason for hiding this comment

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

Prettier handles this, updates with every edit when you save, no need to update manually. Also, the spacing was previously being done manually, but using instead of -

| **express** | [express](https://www.npmjs.org/package/express) |
| **expressDevice** | [express-device](https://www.npmjs.org/package/express-device) |
| **methodOverride** | [method-override](https://www.npmjs.org/package/method-override) |
Expand All @@ -166,35 +160,34 @@ All of the files under the `src/` directory are made available when this module

#### Reusable

| Name | Source | Description |
| :---- | :---- | :---- |
| Name | Source | Description |
| :------------------------- | :---------------------------------------------- | :---------------------------------------------------------------------------------------------------------- |
| **BaseController** | [code](src/webserver/BaseController.js) | A base class in order to be able to identify all of the controllers derived from it. |
| **BaseWebServer** | [code](src/webserver/BaseWebServer.js) | A base web server that sets all of the middleware mentioned here. |
| **ControllerRegistration** | [code](src/webserver/ControllerRegistration.js) | Registers all of the controllers based on the BaseController type and also files that end with `Controller` |
| **BaseMiddleware** | [code](src/middleware/BaseMiddleware.js) | A base class in order to be able to identify all of the middleware derived from it. |

#### Used internally, but can be used/replaced

| Name | Source | Description |
| :---- | :---- | :---- |
| **HtmlErrorRender** | [code](src/handlers/HtmlErrorRender.js) | Sets basic error rendering for uncaught errors. |
| **DefaultMiddleware** | [code](src/middleware/DefaultMiddleware.js) | Registers default express middleware: cookie parser, body parser, method override, and express device |
| **ErrorMiddleware** | [code](src/middleware/ErrorMiddleware.js) | Adds error handling for unhandled errors for requests. |
| **NoCacheMiddleware** | [code](src/middleware/NoCacheMiddleware.js) | Middleware for no cache headers |
| Name | Source | Description |
| :---------------------------------- | :-------------------------------------------------------- | :-------------------------------------------------------------------------------------------------------------------------- |
| **HtmlErrorRender** | [code](src/handlers/HtmlErrorRender.js) | Sets basic error rendering for uncaught errors. |
| **DefaultMiddleware** | [code](src/middleware/DefaultMiddleware.js) | Registers default express middleware: cookie parser, body parser, method override, and express device |
| **ErrorMiddleware** | [code](src/middleware/ErrorMiddleware.js) | Adds error handling for unhandled errors for requests. |
| **NoCacheMiddleware** | [code](src/middleware/NoCacheMiddleware.js) | Middleware for no cache headers |
| **PromiseMiddleware** | [code](src/middleware/PromiseMiddleware.js) | Extends the response object with functionality to be used through promises. It unwraps promises as they are being resolved. |
| **WinstonRequestLoggingMiddleware** | [code](src/middleware/WinstonRequestLoggingMiddleware.js) | Winston middleware for logging every request to the console log. |

| **WinstonRequestLoggingMiddleware** | [code](src/middleware/WinstonRequestLoggingMiddleware.js) | Winston middleware for logging every request to the console log. |

# Contributing

## We accept pull requests

Please send in pull requests and they will be reviewed in a timely manner. Please review this [generic guide to submitting a good pull requests](https://github.com/blog/1943-how-to-write-the-perfect-pull-request). The only things we ask in addition are the following:

* Please submit small pull requests
* Provide a good description of the changes
* Code changes must include tests
* Be nice to each other in comments. :innocent:
- Please submit small pull requests
- Provide a good description of the changes
- Code changes must include tests
- Be nice to each other in comments. :innocent:

## Style guide

Expand Down
2 changes: 1 addition & 1 deletion example/src/config/default.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module.exports = function () {
return this.properties({
Type: 'default',
Port: 9000
Port: 9000,
});
};
2 changes: 1 addition & 1 deletion example/src/config/development.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ module.exports = function () {
this.extends('default');

return this.properties({
Type: 'development'
Type: 'development',
});
};
4 changes: 1 addition & 3 deletions example/src/controllers/HelloController.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
module.exports = function (BaseController) {
class HelloController extends BaseController {

configure(app) {
super.configure(app);

Expand All @@ -14,12 +13,11 @@ module.exports = function (BaseController) {

getHello(req, res) {
const model = {
user: req.query.user || 'John Doe'
user: req.query.user || 'John Doe',
};

res.render('hello', model);
}

}

return new HelloController();
Expand Down
6 changes: 1 addition & 5 deletions example/src/injector.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,11 @@ module.exports = function () {
// Register configuration
registerConfig(ioc, path.join(__dirname, './config'));


ioc.merge(spurCommon());
ioc.merge(spurWeb());

// register folders in your project to be auto-injected
ioc.registerFolders(__dirname, [
'controllers/',
'runtime/'
]);
ioc.registerFolders(__dirname, ['controllers/', 'runtime/']);

return ioc;
};
2 changes: 0 additions & 2 deletions example/src/runtime/WebServer.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
module.exports = function (BaseWebServer) {
class WebServer extends BaseWebServer {

// Add additional changes to the middleware by overriding the method
registerDefaultMiddleware() {
super.registerDefaultMiddleware();
}

}

// Assure there is just one instance
Expand Down
3 changes: 1 addition & 2 deletions example/start.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ injector().inject(function (UncaughtHandler, WebServer, Logger, config, configLo
Logger.info(`PORT: ${config.Port}`);
Logger.info(`CONFIG: ${configLoader.configName}`);

WebServer.start()
.then(() => {
WebServer.start().then(() => {
// Execute other logic after the server has started
});
});
6 changes: 3 additions & 3 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ module.exports = {
branches: 62,
functions: 84,
lines: 89,
statements: 89
}
statements: 89,
},
},
maxWorkers: 1,
moduleFileExtensions: ['js','json'],
moduleFileExtensions: ['js', 'json'],
rootDir: '.',
testEnvironment: 'node',
testMatch: ['**/*.spec.js'],
Expand Down
16 changes: 16 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
"eslint": "8.42.0",
"eslint-plugin-node": "11.1.0",
"jest": "29.7.0",
"prettier": "3.2.5",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just realized that this is another dependency we will need to maintain. I also often see warnings from them that require constant version updates.

Should we reconsider?

Copy link
Contributor Author

@otrreeves otrreeves Apr 4, 2024

Choose a reason for hiding this comment

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

It's just a dev dependency so I would think it's easy to maintain especially with renovate doing the heavy-lifting

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's the publishing it, then publishing harness-web steps.
Not heavy lifting but adds a distraction to a set of packages going into maintenance.

"spur-common": "3.0.2",
"spur-config": "2.0.4"
}
Expand Down
1 change: 0 additions & 1 deletion src/handlers/HtmlErrorRender.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
module.exports = function () {
class HtmlErrorRender {

static render(err, req, res) {
return new HtmlErrorRender().render(err, req, res);
}
Expand Down
8 changes: 2 additions & 6 deletions src/injector.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,10 @@ module.exports = function injector() {
methodOverride,
cookieParser,
bodyParser,
expressWinston
expressWinston,
});

ioc.registerFolders(__dirname, [
'handlers',
'middleware',
'webserver'
]);
ioc.registerFolders(__dirname, ['handlers', 'middleware', 'webserver']);

return ioc;
};
4 changes: 1 addition & 3 deletions src/middleware/BaseMiddleware.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
module.exports = function (
Logger
) {
module.exports = function (Logger) {
class BaseMiddleware {
configure(app) {
this.app = app;
Expand Down
10 changes: 1 addition & 9 deletions src/middleware/DefaultMiddleware.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
module.exports = function (
bodyParser,
cookieParser,
methodOverride,
expressDevice,
BaseMiddleware
) {
module.exports = function (bodyParser, cookieParser, methodOverride, expressDevice, BaseMiddleware) {
class DefaultMiddleware extends BaseMiddleware {

configure(app) {
super.configure(app);

Expand All @@ -16,7 +9,6 @@ module.exports = function (
this.app.use(methodOverride());
this.app.use(expressDevice.capture());
}

}

return new DefaultMiddleware();
Expand Down
Loading
Loading