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(plugin-webpack): adjust publicPath in renderer only #1035

Merged
merged 4 commits into from
Jul 17, 2019

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