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: add support for forge.config.ts et. al #2993

Merged
merged 1 commit into from
Oct 26, 2022
Merged

Conversation

MarshallOfSound
Copy link
Member

This brings in rechoir and interpret which allow arbitrary extensions / loaders for our forge config.

Notably this allows us to make forge.config.ts a thing (and thus generate type safe configurations).

This also splits our ForgeConfig type into two types ForgeConfig and ResolvedForgeConfig.

  • ForgeConfig is the partial config definition that a user provides
  • ResolvedForgeConfig is the full config that is passed around internally and populated with defaults

This was done so that ForgeConfig could be used by apps for their config files. E.g.

import { ForgeConfig } from '@electron-forge/shared-types';

const config: ForgeConfig = {
  // config goes here
};

export default config;

@MarshallOfSound MarshallOfSound requested a review from a team October 26, 2022 08:38
@codecov
Copy link

codecov bot commented Oct 26, 2022

Codecov Report

Merging #2993 (493368c) into main (493368c) will not change coverage.
The diff coverage is n/a.

❗ Current head 493368c differs from pull request most recent head cc9df7f. Consider uploading reports for the commit cc9df7f to get more accurate results

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2993   +/-   ##
=======================================
  Coverage   74.19%   74.19%           
=======================================
  Files          67       67           
  Lines        2178     2178           
  Branches      436      436           
=======================================
  Hits         1616     1616           
  Misses        355      355           
  Partials      207      207           

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 493368c...cc9df7f. Read the comment docs.

Copy link
Member

@VerteDinde VerteDinde left a comment

Choose a reason for hiding this comment

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

Since we've been working on docs, would it be helpful to also have an example of a forge.config.ts that users could reference? That might be nice to create while it's top of mind

@MarshallOfSound
Copy link
Member Author

Since we've been working on docs, would it be helpful to also have an example of a forge.config.ts that users could reference? That might be nice to create while it's top of mind

My plan was to do that and change the webpack-typescript template to use forge.config.ts once this lands

@MarshallOfSound MarshallOfSound force-pushed the ts-config branch 2 times, most recently from 8d83840 to e127433 Compare October 26, 2022 18:52
This brings in rechoir and interpret which allow arbitrary extensions / loaders for our forge config.

Notably this allows us to make forge.config.ts a thing (and thus generate type safe configurations).
@MarshallOfSound MarshallOfSound merged commit e404bf1 into main Oct 26, 2022
@MarshallOfSound MarshallOfSound deleted the ts-config branch October 26, 2022 19:34
@SpacingBat3
Copy link
Contributor

FYI, this was actually a thing in my project even before it made it there. You might find that interesting as an example how I did that previously (and how it was possible to do so) and probably continue doing that.

I had to patch some internal types through, to get more accurate types when comparing it with Electron Forge documentation.

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