Skip to content

fix(server): check for external urls in array - #1980

Merged
hiroppy merged 4 commits into
webpack:masterfrom
EslamHiko:handle-array-paths
Jul 1, 2019
Merged

fix(server): check for external urls in array#1980
hiroppy merged 4 commits into
webpack:masterfrom
EslamHiko:handle-array-paths

Conversation

@EslamHiko

Copy link
Copy Markdown
Contributor
  • This is a bugfix
  • This is a feature
  • This is a code refactor
  • This is a test update
  • This is a docs update
  • This is a metadata update

For Bugs and Features; did you add new tests?

Yes

Motivation / Use-Case

fixes : #1879

Breaking Changes

no

Additional Info

Tests pass then errors show up 😕 maybe because I use windows, let's see will CI be green or not.

@codecov

codecovBot commented Jun 6, 2019

Copy link
Copy Markdown

Codecov Report

Merging #1980 into master will decrease coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@ Coverage Diff @@## master #1980 +/- ##
==========================================
- Coverage 92.8% 92.73% -0.08% 
==========================================
Files 31 31 Lines 1181 1183 +2 Branches 329 328 -1 ==========================================
+ Hits 1096 1097 +1 - Misses 81 82 +1 
Partials 4 4
Impacted FilesCoverage Δ
lib/Server.js92.74% <100%> (+0.01%)⬆️
lib/utils/createConfig.js91.89% <100%> (+0.07%)⬆️
client-src/default/utils/reloadApp.js95.65% <0%> (-4.35%)⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a732303...2ac39d3. Read the comment docs.

@alexander-akaitalexander-akait left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Two notes

Comment threadtest/ExternalPathsWatch.test.js Outdated
@@ -0,0 +1,91 @@
'use strict';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Comment threadtest/ExternalPathsWatch.test.js Outdated
});

describe('testing single & multiple internal paths', () => {
it('Should not throw exception (single line)', (done) => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

single line -> string

Comment threadtest/ContentBase.test.js Outdated
});
});

describe('testing single & multiple internal paths', () => {

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

tests in this part are not necessary, all other tests already test with valid contentBase.

@EslamHiko

Copy link
Copy Markdown
ContributorAuthor

@evilebottnawi since we have now is-absolute-url what about using it instead of Regex?

@alexander-akait

Copy link
Copy Markdown
Member

@EslamHiko yes, will be great 👍

@alexander-akaitalexander-akait left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@alexander-akait

Copy link
Copy Markdown
Member

Sorry for delay

@hiroppy
hiroppy merged commit fa78347 into webpack:masterJul 1, 2019
@hiroppy

Copy link
Copy Markdown
Contributor

Thanks!

@alexander-akait
alexander-akait deleted the handle-array-paths branch July 1, 2019 16:09
@EslamHiko

Copy link
Copy Markdown
ContributorAuthor

@evilebottnawi no problems 😃. @hiroppy My pleasure 😄.

knagaitsev pushed a commit to knagaitsev/webpack-dev-server that referenced this pull request Jul 31, 2019
* fix: check for external urls in array
* test: move tests to contentBase
* fix: use is-absolute-url & add test case for number type
Sign up for freeto 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.

[bug] handle remote content base paths in array

3 participants

@EslamHiko@alexander-akait@hiroppy