Use Config file instead of Environment Variables

[AlbertoBrusa]

Move modular configuration away from env variables and into a configuration file

• Read .modular.js config in modular project root to load up configuration
• Allow env variables to override configuration fill
• Allow use of config where ENV Variables are currently used:
  • USE_MODULAR_ESBUILD
  • USE_MODULAR_WEBPACK
  • Remove USE_MODULAR_WEBPACK
  • EXTERNAL_CDN_TEMPLATE
  • EXTERNAL_ALLOW_LIST
  • EXTERNAL_BLOCK_LIST
  • PUBLIC_URL
  • GENERATE_SOURCEMAP
    • Even where we don't currently use the ENV
• Test:
  • Reading config file
  • ENV variable override
• Change default CDN to esm.sh
• Document configuring via configuration file
  • Document USE_MODULAR_ESBUILD only affects view/esm-view
  • CDN_TEMPLATE only affects esm views
  • Discourage allow/deny lists
• Change all references to env variables in documentation to mention config file
• Edit 4.0.0 changes documentation

We have some of the ENV variables read in .js files that can't use our TS config logic for some reason, so we'll set an ENV variable prefixed with INTERNAL_ for those files to read

[changeset-bot]

🦋 Changeset detected

Latest commit: f0e2e93

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
modular-scripts	Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[coveralls]

Coverage increased (+0.8%) to 25.556% when pulling f0e2e93 on AlbertoBrusa:config into 2e87968 on jpmorganchase:feature/v4.

[cristiano-belloni]

@AlbertoBrusa - While we're at it, can we set EXTERNAL_CDN_TEMPLATE's default to 'https://esm.sh/[name]@[resolution]' instead of Skypack?

[AlbertoBrusa]

Not sure in the slightest why that popped up now but it was complaining for missing typings for lodash

[AlbertoBrusa]

letting getConfig return undefined would have been messy, but treating '' (empty string) as configuration not set might not be ideal either, open to suggestions

[AlbertoBrusa]

The as boolean is safe, as we know for sure what getConfig for each configuration field will return

[AlbertoBrusa]

Not enthusiastic about passing even more ENV variables instead of doing things properly but with importing ts in js not working, this is the easiest solution within the scope of this PR

[sgb-io]

Will resume my review tomorrow, rather than keeping this open until then

[sgb-io]

We should update the wording here now IMO. It's a bit confusing saying "there is a set of minimum configuration required" and then going on to talk about some optional configuration.

Maybe something like:

Modular has minimal configuration because of its philosophy, but some behaviours can be controlled.

[AlbertoBrusa]

Completely agree, I've attempted to re-word it to reflect what this page now contains, let me know what you think

[sgb-io]

I think "Avoid using this unless absolutely necessary" is a bit vague. Can we give some clearer guidance?

Perhaps something like

It is not recommended to change the default value, but this option exists to allow special handling of certain dependencies, e.g. `react`.

[AlbertoBrusa]

Good point, I've now tried to add some details and linked the documentation page containing all details regarding CDN Limitations

[steveukx]

By using generics in this function you can type the return so you don't need to cast the return type in the caller:

interface Config {
  a: string | null;
  b: number | null;
}

type ConfigDefs = {
  [Key in keyof Config]: {
    default: NonNullable<Config[Key]>;
    override: undefined | NonNullable<Config[Key]>;
  }
}

const defs: ConfigDefs = {
  a: {
    default: 'a',
    override: undefined,
  },
  b: {
    default: 1,
    override: 2,
  },
};

function getConfig<T extends keyof ConfigDefs>(key: T): NonNullable<Config[T]> {
  return defs[key].override ?? defs[key].default;
}

[AlbertoBrusa]

Thanks! I was hoping there was a clever way of doing that, looks much cleaner now. Learn something new every day.

[sgb-io]

Everything else LGTM