Skip to content
This repository has been archived by the owner on Jul 9, 2018. It is now read-only.

feat: Add @wordpress/browserslist-config package #69

Merged
merged 5 commits into from
Jan 26, 2018
Merged

feat: Add @wordpress/browserslist-config package #69

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
29a1916
feat: Add `@wordpress/browserslist-config` package
ntwb Jan 23, 2018
b690cb3
Merge branch 'master' into browserslist-config-wordpress
ntwb Jan 26, 2018
398fae0
docs: fix spelling in CHANGELOG.md and consistent formatting
ntwb Jan 26, 2018
0dfaed0
chore: remove `browserslist` from `peerDependencies`
ntwb Jan 26, 2018
87ca228
tests(browserslist-config): refactor test per coding standards
ntwb Jan 26, 2018
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
17 changes: 17 additions & 0 deletions packages/browserslist-config/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# 2.0.0

- Transfer to @wordpress npm orginization
Copy link
Member

Choose a reason for hiding this comment

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

orginization -> organization.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @atimmer 👍 Fixed in 398fae0

- Published as @wordpress/browserslist-config

# 1.1.0

- Add browserslist tests
- Refactor per coding standards

# 1.0.1

- Update installation documentation.

# 1.0.0

- Initial release.
21 changes: 21 additions & 0 deletions packages/browserslist-config/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# @wordpress/browserslist-config

[WordPress Browserslist](https://make.wordpress.org/design/handbook/design-guide/browser-support/) shareable config for [Browserslist](https://www.npmjs.com/package/browserslist).

## Installation

Install the module

```shell
$ npm install @wordpress/browserslist-config
```

## Usage

Add this to your `package.json` file:

```json
"browserslist": [
"extends @wordpress/browserslist-config"
]
```
13 changes: 13 additions & 0 deletions packages/browserslist-config/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// browserslist-config/index.js
module.exports = [
'> 1%',
'ie >= 11',
'last 1 Android versions',
'last 1 ChromeAndroid versions',
'last 2 Chrome versions',
'last 2 Firefox versions',
'last 2 Safari versions',
'last 2 iOS versions',
'last 2 Edge versions',
'last 2 Opera versions',
];
30 changes: 30 additions & 0 deletions packages/browserslist-config/package-lock.json
View file
Open in desktop

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

33 changes: 33 additions & 0 deletions packages/browserslist-config/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
{
"name": "@wordpress/browserslist-config",
"version": "2.0.0",
"description": "WordPress Browserslist Shared Config",
"author": "WordPress",
"license": "GPL-2.0-or-later",
"keywords": [
"wordpress",
"browserslist",
"browserslist-config"
],
"homepage": "https://github.com/WordPress/packages/tree/master/packages/browserslist-config/README.md",
"repository": {
"type": "git",
"url": "https://github.com/WordPress/packages.git"
},
"bugs": {
"url": "https://github.com/WordPress/packages/issues"
},
"main": "index.js",
"engines": {
"node": ">=8"
},
"devDependencies": {
"browserslist": "^2.11.3"
},
"peerDependencies": {
Copy link
Member

Choose a reason for hiding this comment

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

Is it necessary since it is already listed as devDependencies? Or devDependencies should be dependencies? I never know :)

Copy link
Member Author

Choose a reason for hiding this comment

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

As browserslist is only used in the Jest tests it only requires to be included in devDependencies, that said, here's why it is included:

I added it to peerDependencies in ntwb/browserslist-config-wordpress@8c6f1b1 because of eslint-plugin-node's no-unpublished-require rule.

I opened an issue a while ago here asking that devDependencies modules be ignored from these rules and the end result was not what I had hoped for, hence the addition of browserslist to peerDependencies works as a workaround.

These are the two ESLint errors thrown when browserslist is not included in peerDependencies

  1:31  error  "browserslist" is not found                       node/no-missing-require
  1:31  error  "browserslist" is not published                   node/no-unpublished-require

I'm going to remove it from peerDependencies and add an .eslintrc.json in the test folder to ignore those two eslint-plugin-node/recommended rules as suggested here which I think is not quite as hacky...

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed it 0dfaed0, will add the . eslintrc.json when adding ESLint in, I'm tempted to turn both those rules off by default config now.

"browserslist": "^2.11.3"
},
"publishConfig": {
"access": "public"
}
}
17 changes: 17 additions & 0 deletions packages/browserslist-config/test/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
const browserslist = require( 'browserslist' );
Copy link
Member

@gziolo gziolo Jan 25, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, the whole file should follow WordPress spacing rules.

const config = require( '../' );

beforeEach(() => {
jest.resetModules();
});

it( 'should export an array', () => {
expect( Array.isArray( config ) ).toBe( true );
});

it( 'should not contain invalid queries', () => {
jest.doMock( '@wordpress/browserslist-config', () => require( '../index' ), { virtual: true });

const result = browserslist(['extends @wordpress/browserslist-config']);
Copy link
Member

Choose a reason for hiding this comment

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

Spacing should be fixed to match coding styles.

Copy link
Member Author

@ntwb ntwb Jan 26, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I'm going to skip this change, for now, we need to get ESLint setup

The blocker to this is getting the changes from https://make.wordpress.org/core/2017/09/07/proposal-for-js-standards-revision-removing-array-function-whitespace-exceptions/ merged.

In me hacking in ESLint to run over this PR both the following return no ESLint errors:
• const result = browserslist([ 'extends @wordpress/browserslist-config' ]);
• const result = browserslist(['extends @wordpress/browserslist-config']);

And this does:
• const result = browserslist( ['extends @wordpress/browserslist-config'] );

I'm not sure why space-in-parens ESLint rule is throwing that, I could look now but instead, I'll get it fixed over the weekend, or early next week, a public holiday here today so I'm going to head out for a bit 🏖

expect( result ).toBeTruthy();
});