-
Notifications
You must be signed in to change notification settings - Fork 68
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
Use Config file instead of Environment Variables #2227
Changes from 23 commits
2959334
e1571dd
dac6289
7fb5370
33742bf
cde234d
dc77942
cec53a1
f854b5e
249390f
7e08f6a
b9b4f5b
675b10f
73fee89
b08bd37
5054f28
755a13f
1a447a3
ee938f7
7eead44
5a94077
142b7ef
c085182
c8ef95c
6175809
f73878e
7448a3b
f0e2e93
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
--- | ||
'modular-scripts': major | ||
--- | ||
|
||
Changed default CDN from Skypack to esm.sh as skypack is no longer actively | ||
maintained. Add support for configuring modular through a configuration file. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
module.exports = { | ||
useModularEsbuild: true, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,7 +46,9 @@ const resolveModular = (relativePath) => | |
const publicUrlOrPath = getPublicUrlOrPath( | ||
process.env.NODE_ENV === 'development', | ||
require(resolveApp('package.json')).homepage, | ||
process.env.PUBLIC_URL, | ||
process.env.INTERNAL_PUBLIC_URL === '' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. letting getConfig return undefined would have been messy, but treating '' (empty string) as configuration not set might not be ideal either, open to suggestions |
||
? undefined | ||
: process.env.INTERNAL_PUBLIC_URL, | ||
); | ||
|
||
const buildPath = path.join(modularRoot, 'dist', modularPackageName); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
// Jest Snapshot v1, https://goo.gl/fbAQLP | ||
|
||
exports[`modular-scripts WHEN building a esm-view THEN matches the entrypoint snapshot 1`] = ` | ||
"import * as t from "https://cdn.skypack.dev/[email protected]"; | ||
"import * as t from "https://esm.sh/[email protected]"; | ||
function e() { | ||
return t.createElement( | ||
"div", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
import execa from 'execa'; | ||
import { copyFileSync } from 'fs'; | ||
import path from 'path'; | ||
import { createModularTestContext } from '../../test/utils'; | ||
import getModularRoot from '../../utils/getModularRoot'; | ||
|
||
const modularRoot = getModularRoot(); | ||
const configFixtures = path.join(modularRoot, '__fixtures__', 'test-config'); | ||
|
||
/** | ||
* Run modular with provided arguments in specified directory | ||
*/ | ||
function modular( | ||
args: string, | ||
cwd: string, | ||
opts: Record<string, unknown> = {}, | ||
) { | ||
return execa('yarnpkg', ['modular', ...args.split(' ')], { | ||
cwd, | ||
cleanup: true, | ||
...opts, | ||
}); | ||
} | ||
|
||
// Temporary test context paths set by createTempModularRepoWithTemplate() | ||
let tempModularRepo: string; | ||
|
||
describe('A simple modular repo with a .modular.js config file', () => { | ||
beforeEach(async () => { | ||
tempModularRepo = createModularTestContext(); | ||
await modular('add test-app --unstable-type app', tempModularRepo); | ||
copyFileSync( | ||
path.join(configFixtures, '.modular.js'), | ||
path.join(tempModularRepo, '.modular.js'), | ||
); | ||
}); | ||
it('builds using esbuild as specified in config file', async () => { | ||
const result = await modular(`build test-app --verbose`, tempModularRepo); | ||
expect(result.stdout).toContain('Building with esbuild'); | ||
expect(result.exitCode).toBe(0); | ||
}); | ||
it('builds using webpack if the environment variable is provided as it overrides the config', async () => { | ||
const result = await modular(`build test-app --verbose`, tempModularRepo, { | ||
env: { | ||
USE_MODULAR_ESBUILD: 'false', | ||
}, | ||
}); | ||
expect(result.stdout).toContain('Building with Webpack'); | ||
expect(result.exitCode).toBe(0); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ import getRelativeLocation from '../../utils/getRelativeLocation'; | |
import createEsbuildBrowserslistTarget from '../../utils/createEsbuildBrowserslistTarget'; | ||
|
||
import type { ModularPackageJson } from '@modular-scripts/modular-types'; | ||
import { getConfig } from '../../utils/config'; | ||
|
||
const outputDirectory = 'dist'; | ||
const extensions = ['.ts', '.tsx', '.js', '.jsx']; | ||
|
@@ -107,7 +108,7 @@ export async function makeBundle( | |
|
||
const outputOptions: rollup.OutputOptions = { | ||
freeze: false, | ||
sourcemap: true, // TODO: read this off env | ||
sourcemap: getConfig('generateSourceMap') as boolean, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
sourcemapPathTransform(relativeSourcePath: string, sourceMapPath: string) { | ||
// make source map input files relative to the `${packagePath}/dist-${format}` within | ||
// the package directory | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure in the slightest why that popped up now but it was complaining for missing typings for lodash