Skip to content

fix(client): default client fallback with redirection to all default directory files - #2049

Closed
knagaitsev wants to merge 3 commits into
webpack:masterfrom
knagaitsev:client-definition-fix-2
Closed

fix(client): default client fallback with redirection to all default directory files#2049
knagaitsev wants to merge 3 commits into
webpack:masterfrom
knagaitsev:client-definition-fix-2

Conversation

@knagaitsev

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?

Motivation / Use-Case

Fix#2006,

Fix problems introduced by #2015

I made a redirect to each file in the default directory, because it's better to be safe I think.

Breaking Changes

None

Additional Info

@codecov

codecovBot commented Jun 18, 2019

Copy link
Copy Markdown

Codecov Report

Merging #2049 into master will decrease coverage by 0.66%.
The diff coverage is n/a.

Impacted file tree graph

@@ Coverage Diff @@## master #2049 +/- ##
==========================================
- Coverage 92.77% 92.11% -0.67% 
==========================================
Files 29 20 -9 Lines 1149 875 -274 Branches 327 273 -54 ==========================================
- Hits 1066 806 -260 + Misses 79 66 -13 + Partials 4 3 -1
Impacted FilesCoverage Δ
client-src/default/utils/createSocketUrl.js
client-src/clients/SockJSClient.js
client-src/default/utils/reloadApp.js
client-src/default/utils/getCurrentScriptSource.js
client-src/clients/BaseClient.js
client-src/default/utils/log.js
client-src/default/index.js
client-src/default/utils/sendMessage.js
client-src/default/overlay.js

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 26211fc...2e8e7d5. Read the comment docs.

@knagaitsev

Copy link
Copy Markdown
ContributorAuthor

@evilebottnawi@hiroppy Here is a fix.

If you revert the previous fix, I will just rebase and fix any problems that result from that.

@knagaitsev

Copy link
Copy Markdown
ContributorAuthor

Also I should add tests to make sure all the redirection are correct.

Would it make more sense if test/client tested files in the client directory, rather than the client-src directory, since client directory is what users will actually use? If we switched to this method, I could simply point those tests towards the redirection files that I create in this PR, and it would confirm that they work.

@knagaitsev

knagaitsev commented Jun 20, 2019

Copy link
Copy Markdown
ContributorAuthor

The change I just made makes the client tests use client directory, rather than client-src. Those are the files that the dev server and users use, so they should be tested. The unit tests pass through the redirection files that I added to each individual client file, confirming that they work.

This fix spans more than I expected, but probably is safer than other solutions.

Edit:

Just to explain more, I changed the tests so that the test mocks will mock the files in the client/default directory, since this is where those files actually interact with one another. In the client directory, we simply have the "redirection" files that do something like module.exports = require('./default/index'). Whenever there is a require in a client unit test, I require those "redirection" files, confirming that redirection works, then those redirection files go on to call the mock files.

@knagaitsev

Copy link
Copy Markdown
ContributorAuthor

Closing in favor of #2069

@knagaitsevknagaitsev added the gsoc Google Summer of Code label Aug 13, 2019
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gsocGoogle Summer of Code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Uncaught ReferenceError: __webpack_dev_server_client__ is not defined

1 participant

@knagaitsev