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.

Sign up for GitHub

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

Jump to bottom

Chore: upgrade dependencies of browser test #14127

Merged
merged 5 commits into from
Mar 5, 2021
Merged

Chore: upgrade dependencies of browser test #14127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
88af91a
Chore: upgrade dependencies of browser test
g-plane Feb 21, 2021
dc02687
Chore: simplify Node.js modules polyfill
g-plane Feb 24, 2021
e262102
Chore: remove unnecessary dependency
g-plane Mar 2, 2021
6189424
Chore: simplify webpack config in Karma
g-plane Mar 2, 2021
b0e3f18
Chore: clean up entries in Karma config
g-plane Mar 4, 2021
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
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ jobs:
steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1
with:
node-version: '12'
g-plane marked this conversation as resolved.
Show resolved Hide resolved
- name: Install Packages
run: npm install
- name: Test
Expand Down
12 changes: 10 additions & 2 deletions karma.conf.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"use strict";
const os = require("os");
const NodePolyfillPlugin = require("node-polyfill-webpack-plugin");

if (os.arch() === "arm64") {

Expand All @@ -20,12 +21,11 @@ module.exports = function(config) {
* frameworks to use
* available frameworks: https://npmjs.org/browse/keyword/karma-adapter
*/
frameworks: ["mocha"],
frameworks: ["mocha", "webpack"],


// list of files / patterns to load in the browser
files: [
"build/eslint.js",
"tests/lib/linter/linter.js"
],

Expand All @@ -44,6 +44,14 @@ module.exports = function(config) {
},
webpack: {
mode: "none",
plugins: [
Copy link
Member

Choose a reason for hiding this comment

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

is the config any difference with ./webpack.config.js? if no, we can just import it here:

webpack: require("./webpack.config.js"),

Copy link
Member Author

Choose a reason for hiding this comment

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

The webpack configuration in root is for building ESLint as a library, while in Karma, it's for browser testing as an application.

new NodePolyfillPlugin()
],
resolve: {
alias: {
"../../../lib/linter$": "../../../build/eslint.js"
Copy link
Member

Choose a reason for hiding this comment

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

Since we are now bundling build/eslint.js into tests, do we need to load it in the browser (i.e. can we remove it from the files array above)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Are there any pros?

Copy link
Member

Choose a reason for hiding this comment

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

Technically not much if any, but for a better understanding of what's happening here, so that someone who reads the code doesn't have to think about why are we loading the script and where is it used (while it actually isn't used, the code is bundled in another script). Is there a benefit of loading build/eslint.js now?

}
},
stats: "errors-only"
},
webpackMiddleware: {
Expand Down
13 changes: 7 additions & 6 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,18 +108,19 @@
"fs-teardown": "^0.1.0",
"glob": "^7.1.6",
"jsdoc": "^3.5.5",
"karma": "^4.0.1",
"karma": "^6.1.1",
"karma-chrome-launcher": "^3.1.0",
"karma-mocha": "^1.3.0",
"karma-mocha-reporter": "^2.2.3",
"karma-webpack": "^4.0.0-rc.6",
"karma-mocha": "^2.0.1",
"karma-mocha-reporter": "^2.2.5",
"karma-webpack": "^5.0.0",
"lint-staged": "^10.1.2",
"load-perf": "^0.2.0",
"markdownlint": "^0.19.0",
"markdownlint-cli": "^0.22.0",
"memfs": "^3.0.1",
"mocha": "^7.1.1",
"mocha-junit-reporter": "^1.23.0",
"node-polyfill-webpack-plugin": "^1.0.3",
"npm-license": "^0.3.3",
"nyc": "^15.0.1",
"proxyquire": "^2.0.1",
Expand All @@ -129,8 +130,8 @@
"shelljs": "^0.8.2",
"sinon": "^9.0.1",
"temp": "^0.9.0",
"webpack": "^4.35.0",
"webpack-cli": "^3.3.5",
"webpack": "^5.23.0",
"webpack-cli": "^4.5.0",
"yorkie": "^2.0.0"
},
"keywords": [
Expand Down
23 changes: 1 addition & 22 deletions tests/lib/linter/linter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,6 @@

"use strict";

//------------------------------------------------------------------------------
// Helper
//------------------------------------------------------------------------------

/**
* To make sure this works in both browsers and Node.js
* @param {string} name Name of the module to require
* @param {Object} windowName name of the window
* @returns {Object} Required object
* @private
*/
function compatRequire(name, windowName) {
if (typeof window === "object") { // eslint-disable-line no-undef
return window[windowName || name]; // eslint-disable-line no-undef
}
if (typeof require === "function") {
return require(name);
}
throw new Error(`Cannot find object '${name}'.`);
}

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------
Expand All @@ -35,7 +14,7 @@ const assert = require("chai").assert,
esprima = require("esprima"),
testParsers = require("../../fixtures/parsers/linter-test-parsers");

const { Linter } = compatRequire("../../../lib/linter", "eslint");
const { Linter } = require("../../../lib/linter");

//------------------------------------------------------------------------------
// Constants
Expand Down
5 changes: 5 additions & 0 deletions webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
"use strict";
const NodePolyfillPlugin = require("node-polyfill-webpack-plugin");

/** @type {import("webpack").Configuration} */
module.exports = {
mode: "none",
entry: {
Expand Down Expand Up @@ -42,6 +44,9 @@ module.exports = {
}
]
},
plugins: [
new NodePolyfillPlugin()
],
resolve: {
mainFields: ["browser", "main", "module"]
},
Expand Down