Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

shimaore
Copy link
Contributor

@shimaore shimaore commented Jul 17, 2019

This reverts improper PR#1021 and re-implements the changes I have in the JS (compiled) code at the right place.

  • I have read the contribution documentation for this project.
  • I agree to follow the code of conduct that this project follows, as appropriate.
  • The changes are appropriately documented (if applicable).
  • The changes have sufficient test coverage (if applicable).
  • The testsuite passes successfully on my local machine (if applicable).

Summarize your changes:

As pointed out by @malept in #1021 (comment) I had some trouble when trying to map my changes in JavaScript back to TypeScript. This attempts to do the change at the proper location (i.e. inside getRendererConfig).

Fixes #1034.

This reverts improper PR#1021 and re-implements the changes I have in the JS (compiled) code at the right place.
@malept
Copy link
Member

malept commented Jul 17, 2019

Hmmm. I tested this locally, and the CSS file reference doesn't seem to work. Although that may have never worked when packaged with the current config.

@shimaore
Copy link
Contributor Author

Hmmm. I tested this locally, and the CSS file reference doesn't seem to work. Although that may have never worked when packaged with the current config.

@malept not sure this might help, but in order to get import './index.css' in src/renderer.js to work I had to replace file-loader with css-loader (under the test for CSS) in webpack.render.config.js.

@malept
Copy link
Member

malept commented Jul 17, 2019

Do you mind making the change that made it work for you in this PR?

@shimaore
Copy link
Contributor Author

@malept first attempt

This is kind of a mixed bag because it plays both in webpack-plugin and in the webpack template and I have no clue how to properly test all this end-to-end. I'm really guessing at this point.

@shimaore
Copy link
Contributor Author

Hmm FWIW I'm also using style-loader instead of style-loader/url in webpack.renderer.config.js, I'm not sure if that makes a difference.

@malept
Copy link
Member

malept commented Jul 17, 2019

Ah, I see what I was doing wrong. I forgot to add it to package.json 🤦‍♂️

Yeah, we can put that in a different PR. I'll clean this up and merge it.

@malept malept changed the title Revert PR #1021 and re-implement fix(plugin-webpack): adjust publicPath in renderer only Jul 17, 2019
@shimaore
Copy link
Contributor Author

The working combination for CSS is style-loader + css-loader; I'll open a separate PR for that.
I'm not sure how to get it to work with style-loader/url.

@shimaore
Copy link
Contributor Author

The CSS-related changes are now in #1036 .

@malept malept merged commit 57ca285 into electron:master Jul 17, 2019
Sign up for free to 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.

webpack: packaged app has incorrect reference to renderer index.js

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.