forge icon indicating copy to clipboard operation
forge copied to clipboard

chore(plugin-webpack): use trailing slash in dev mode entrypoint baseUrl

Open dsanders11 opened this issue 3 years ago • 2 comments

  • [x] I have read the contribution documentation for this project.
  • [x] 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:

This better reflects the reality that it's a directory, and it makes it easier to construct relative URLs via new URL('foo/', window.location.href) which would otherwise strip the entrypoint name.

dsanders11 avatar Jun 27 '22 02:06 dsanders11

This seems more like a fix: than a chore: to me, but what's your reasoning?

Yea, I went back and forth on which to label it. It's not technically a bug, it's more of an usability improvement. Happy to change it to fix: if you'd prefer that.

Is there a reasonable way to add regression tests for this?

I think so, looks like it's failing this test anyway: "sets the renderer entry point to an HTML file if both an HTML & JS file are specified:" due to the extract slash. I'll write some additional test coverage. Looks like it also broke a couple of tests. I'm guilty of not testing, I fired this off during a larger debugging session on the Fiddle GSoC stuff.

Will move this to draft for the moment and fix it up during the week.

dsanders11 avatar Jun 27 '22 04:06 dsanders11

Codecov Report

Merging #2890 (caef034) into master (067a68c) will decrease coverage by 14.85%. The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #2890       +/-   ##
===========================================
- Coverage   71.40%   56.55%   -14.86%     
===========================================
  Files          79       48       -31     
  Lines        2385     1457      -928     
  Branches      449      295      -154     
===========================================
- Hits         1703      824      -879     
- Misses        454      530       +76     
+ Partials      228      103      -125     
Impacted Files Coverage Δ
packages/api/core/src/util/upgrade-forge-config.ts 10.52% <0.00%> (-87.72%) :arrow_down:
packages/api/core/src/util/publish-state.ts 12.82% <0.00%> (-79.49%) :arrow_down:
packages/api/core/src/util/electron-executable.ts 24.00% <0.00%> (-76.00%) :arrow_down:
packages/maker/snap/src/MakerSnap.ts 40.00% <0.00%> (-60.00%) :arrow_down:
packages/api/core/src/api/start.ts 15.66% <0.00%> (-50.22%) :arrow_down:
packages/maker/wix/src/util/author-name.ts 50.00% <0.00%> (-50.00%) :arrow_down:
packages/maker/appx/src/util/author-name.ts 50.00% <0.00%> (-50.00%) :arrow_down:
packages/api/core/src/api/publish.ts 18.42% <0.00%> (-50.00%) :arrow_down:
packages/maker/flatpak/src/MakerFlatpak.ts 33.33% <0.00%> (-43.94%) :arrow_down:
packages/api/core/src/util/electron-version.ts 32.30% <0.00%> (-43.08%) :arrow_down:
... and 58 more

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 067a68c...caef034. Read the comment docs.

codecov[bot] avatar Jun 29 '22 07:06 codecov[bot]