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

feat: remove preload.js from TypeScript templates #2938

Conversation

itsananderson
Copy link
Member

  • 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:

When preload was added to the Forge templates, the TypeScript templates actually ended up with both a preload.js and a preload.ts because the base template adds preload.js and the TypeScript template adds a preload.ts. This PR modifies the TS templates to remove the preload.js just like it already removes the index.js.

I also modified the template tests to verify that no js files end up in the src/ folder after the TypeScript templates are applied. That way if any other new files are added in the future, the tests will automatically verify that only the TypeScript versions are applied.

@itsananderson itsananderson changed the title feat: Remove preload.js from TypeScript templates feat: remove preload.js from TypeScript templates Sep 17, 2022
@codecov
Copy link

codecov bot commented Sep 17, 2022

Codecov Report

Merging #2938 (195bd86) into master (cb1e560) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2938      +/-   ##
==========================================
+ Coverage   71.32%   71.33%   +0.01%     
==========================================
  Files          79       79              
  Lines        2413     2414       +1     
  Branches      452      452              
==========================================
+ Hits         1721     1722       +1     
  Misses        469      469              
  Partials      223      223              
Impacted Files Coverage Δ
...ages/template/typescript/src/TypeScriptTemplate.ts 100.00% <100.00%> (ø)

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 cb1e560...195bd86. Read the comment docs.

@erickzhao
Copy link
Member

Some of this seems to be implemented in #2936 but the tests are a great touch.

@itsananderson
Copy link
Member Author

Ah cool I saw an earlier iteration of that PR that didn't have the removal of preload.js. I'll just rework this PR to be about updating tests.

@itsananderson
Copy link
Member Author

Turns out #2936 only fixed TypeScriptWebpackTemplate.ts and not TypeScriptTemplate.ts so after merging with master this PR is roughly the same as before. The edit to TypeScriptWebpackTemplate.ts simply became redundant and was removed.

@erickzhao erickzhao self-requested a review September 20, 2022 17:03
@VerteDinde VerteDinde merged commit 50484dc into electron:master Sep 28, 2022
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.

3 participants