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

fix env #284

Merged
merged 18 commits into from
Dec 10, 2021
5 changes: 5 additions & 0 deletions changelog.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# changelog

## 0.45.2

- fix `process.env.NODE_ENV` for production tasks
([#283](https://github.com/feltcoop/gro/pull/283))

## 0.45.1

- fix peer dependency versions
Expand Down
2 changes: 1 addition & 1 deletion src/cli/invoke.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const main = async () => {

// install sourcemaps for Gro development
if (process.env.NODE_ENV !== 'production') {
const sourcemapSupport = await import('source-map-support');
const sourcemapSupport = await import('source-map-support'); // is a peer dependency
sourcemapSupport.install({
handleUncaughtExceptions: false,
});
Expand Down
9 changes: 3 additions & 6 deletions src/config/gro.config.default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,9 @@ This is the default config that's passed to `src/gro.config.ts`
if it exists in the current project, and if not, this is the final config.
It looks at the project and tries to do the right thing:

- if `src/routes` and `src/app.html`,
assumes a SvelteKit frontend
- if `src/lib/index.ts`,
assumes a Node library
- if `src/lib/server/server.ts`,
assumes a Node API server
- if `src/routes` and `src/app.html`, assumes a SvelteKit frontend
- if `src/lib/index.ts`, assumes a Node library
- if `src/lib/server/server.ts`, assumes a Node API server

*/

Expand Down
3 changes: 2 additions & 1 deletion src/deploy.task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ export const task: Task<TaskArgs> = {

try {
// Run the build.
await invokeTask('build', {...args, clean: false});
// TODO should this `spawn` and set `NODE_ENV`?
ryanatkn marked this conversation as resolved.
Show resolved Hide resolved
await invokeTask('build');

// After the build is ready, set the deployed directory, inferring as needed.
if (dirname !== undefined) {
Expand Down
2 changes: 1 addition & 1 deletion src/docs/task.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ and defers composition to the user in regular TypeScript modules.

- Gro automatically discovers [all `*.task.ts` files](../docs/tasks.md)
in your source directory, so creating a new task is as simple as creating a new file -
no configuration or scaffolding commands needed!
no configuration needed
- task definitions are just objects with an async `run` function and some optional properties,
so composing tasks is explicit in your code, just like any other module
(but there's also the helper `invokeTask`: see more below)
Expand Down
2 changes: 1 addition & 1 deletion src/publish.task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export const task: Task<TaskArgs> = {
}

// Build to create the final artifacts:
await invokeTask('build', {...args, clean: false});
await invokeTask('build');

if (dry) {
log.info({versionIncrement, publish: config.publish, branch});
Expand Down
40 changes: 39 additions & 1 deletion src/task/invokeTask.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {EventEmitter} from 'events';
import {createStopwatch, Timings} from '@feltcoop/felt/util/timings.js';
import {printMs, printTimings} from '@feltcoop/felt/util/print.js';
import {plural} from '@feltcoop/felt/util/string.js';
import {spawn} from '@feltcoop/felt/util/process.js';

import type {Args} from 'src/task/task.js';
import {runTask} from './runTask.js';
Expand Down Expand Up @@ -47,6 +48,15 @@ The comments describe each condition.

*/

const serializeArgs = (args: Args): string[] => {
const result: string[] = [];
for (const [key, value] of Object.entries(args)) {
result.push(`--${key}`);
result.push((value as any).toString());
}
return result;
};

export const invokeTask = async (
fs: Filesystem,
taskName: string,
Expand All @@ -55,7 +65,35 @@ export const invokeTask = async (
dev?: boolean,
): Promise<void> => {
// TODO not sure about this -- the idea is that imported modules need the updated `NODE_ENV`
process.env['NODE_ENV'] = dev || dev === undefined ? 'development' : 'production';
process.env['NODE_ENV'] =
dev === undefined
? process.env['NODE_ENV'] === 'production'
? 'production'
: 'development'
: dev
? 'development'
: 'production';
// TODO If `dev` doesn't match the `NODE_ENV`,
ryanatkn marked this conversation as resolved.
Show resolved Hide resolved
// run the task in a separate process,
// to ensure all modules have the correct `NODE_ENV` at initialization.
// TODO problem with this design is that events won't work when new processes are spawned,
// but I don't see a better way to ensure running tasks with the correct `NODE_ENV`
// TODO another issue is that this introduces a dependency on `npx`
// to an otherwise abstracted function. but is it needed to ensure
// Can we instead fix this by using `spawn` for `gro build` instead of invoking?
// if (
// dev !== undefined &&
// ((dev && process.env['NODE_ENV'] === 'production') ||
// (!dev && process.env['NODE_ENV'] !== 'production'))
// ) {
// await spawn('npx', ['gro', taskName, ...serializeArgs(args)], {
// env: {
// // TODO include `...process.env`?
// NODE_ENV: dev ? 'development' : 'production',
// },
// });
// return;
// }

const log = new SystemLogger(printLogLabel(taskName || 'gro'));

Expand Down