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

fix(karma-webpack): correctly map entries to outputted assets (config.output) #347

Merged
merged 3 commits into from
Sep 3, 2018

Conversation

ryanclark
Copy link
Collaborator

@ryanclark ryanclark commented Sep 3, 2018

Fixes issue with tests not running due to the filename being the default value ([name].js).

Issue stemmed from the entry files being added and their name being set to the full path, including file extension. This means that webpack would store the file in memory with a double extension (filename.js.js).

This changes the code so the entry file's name no longer includes the extension.

It also fixes the issue with files with the extension .ts (and any other extension, such as .coffee)
having to use karma's mime configuration to run the tests due to webpack outputting a .ts file - this is no longer needed as the entry files are mapped to their outputted asset file from webpack's stats. This way if you use tests/**/*.ts with the default filename setting of [name].js, webpack will output the test files as tests/**/*.js, and update karma to point to the .js files in the browser.

This fixes this comment - #322 (comment) - but I don't think the actual issue is related to the comment.

Type

  • Fix

Issues

SemVer

  • Patch

@ryanclark ryanclark added this to the 4.0.0 milestone Sep 3, 2018
@michael-ciniawsky michael-ciniawsky changed the title fix(karma-webpack): fix issue with tests hanging due to filename config fix(karma-webpack): correctly map entries to their outputted assets (config.output) Sep 3, 2018
@michael-ciniawsky michael-ciniawsky changed the title fix(karma-webpack): correctly map entries to their outputted assets (config.output) fix(karma-webpack): correctly map entries to outputted assets (config.output) Sep 3, 2018
@michael-ciniawsky
Copy link
Contributor

This needs a backport to the 3.0.0 branch aswell

Copy link
Contributor

@michael-ciniawsky michael-ciniawsky left a comment

Choose a reason for hiding this comment

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

If you don't agree with the suggested stylistic changes, feel free to just leave as-is

@@ -111,6 +111,8 @@ function Plugin(
this.files = [];
this.basePath = basePath;
this.waiting = [];
this.entryFilesMap = new Map();
Copy link
Contributor

Choose a reason for hiding this comment

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

this.entries = new Map()
this.outputs = new Map()

@@ -157,6 +159,20 @@ function Plugin(
applyStats.forEach((stats) => {
stats = stats.toJson();

this.outputFilesMap.clear();

const assetKeys = Object.keys(stats.assetsByChunkName);
Copy link
Contributor

Choose a reason for hiding this comment

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

const entries = ...


const assetKeys = Object.keys(stats.assetsByChunkName);
for (let i = 0; i < assetKeys.length; i++) {
const entryName = assetKeys[i];
Copy link
Contributor

Choose a reason for hiding this comment

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

const entry = entries[i]

@michael-ciniawsky michael-ciniawsky changed the title fix(karma-webpack): correctly map entries to outputted assets (config.output) fix(karma-webpack): correctly map entries to outputted assets (config.output) Sep 3, 2018
Copy link
Contributor

@michael-ciniawsky michael-ciniawsky left a comment

Choose a reason for hiding this comment

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

👍

@michael-ciniawsky
Copy link
Contributor

Released in v4.0.0-rc.1 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transpiled file is in .ts, instead of .js.
2 participants