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

Refactor/Improve some typings #909

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ module.exports = {
The `options` object contains the following:

```tsx
export interface TsdxOptions {
export interface TSDXOptions {
// path to file
input: string;
// Name of package
Expand Down Expand Up @@ -375,7 +375,7 @@ module.exports = {

### Babel

You can add your own `.babelrc` to the root of your project and TSDX will **merge** it with [its own Babel transforms](./src/babelPluginTsdx.ts) (which are mostly for optimization), putting any new presets and plugins at the end of its list.
You can add your own `.babelrc` to the root of your project and TSDX will **merge** it with [its own Babel transforms](./src/babelPluginTSDX.ts) (which are mostly for optimization), putting any new presets and plugins at the end of its list.

### Jest

Expand Down
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,12 @@
"typescript": "^3.7.3"
},
"devDependencies": {
"@types/babel__core": "^7.1.10",
"@types/babel__traverse": "^7.0.15",
"@types/eslint": "^6.1.2",
"@types/fs-extra": "^9.0.1",
"@types/lodash": "^4.14.161",
"@types/lodash.merge": "^4.6.6",
"@types/node": "^14.11.1",
"@types/react": "^16.9.11",
"@types/rollup-plugin-json": "^3.0.2",
Expand Down
20 changes: 18 additions & 2 deletions src/babelPluginTsdx.ts → src/babelPluginTSDX.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { createConfigItem } from '@babel/core';
import { createBabelInputPluginFactory } from '@rollup/plugin-babel';
import {
createBabelInputPluginFactory,
RollupBabelInputPluginOptions,
} from '@rollup/plugin-babel';
import { Plugin } from 'rollup';
import merge from 'lodash.merge';

export const isTruthy = (obj?: any) => {
Expand Down Expand Up @@ -48,7 +52,19 @@ export const createConfigItems = (type: any, items: any[]) => {
});
};

export const babelPluginTsdx = createBabelInputPluginFactory(() => ({
// augment with `custom` since it's added in `options` below
type IBabelPluginTSDX = (
options: RollupBabelInputPluginOptions & {
custom: any;
// not sure if @types/babel__core is incorrect or this is actually ignored
passPerPreset: boolean;
}
) => Plugin;

// temp workaround so prettier doesn't reformat 100 lines for indentation
const shortBabelPlugin = createBabelInputPluginFactory;

export const babelPluginTSDX: IBabelPluginTSDX = shortBabelPlugin(() => ({
// Passed the plugin options.
options({ custom: customOptions, ...pluginOptions }: any) {
return {
Expand Down
23 changes: 14 additions & 9 deletions src/createBuildConfigs.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
import { RollupOptions, OutputOptions } from 'rollup';
import * as fs from 'fs-extra';
import { concatAllArray } from 'jpjs';

import { paths } from './constants';
import { TsdxOptions, NormalizedOpts } from './types';
import {
TSDXConfig,
TSDXOptions,
AtLeastOneTSDXOptions,
NormalizedOpts,
RollupOptionsWithOutput,
} from './types';

import { createRollupConfig } from './createRollupConfig';

// check for custom tsdx.config.js
let tsdxConfig = {
rollup(config: RollupOptions, _options: TsdxOptions): RollupOptions {
let tsdxConfig: TSDXConfig = {
rollup(config, _options) {
return config;
},
};
Expand All @@ -20,11 +25,11 @@ if (fs.existsSync(paths.appConfig)) {

export async function createBuildConfigs(
opts: NormalizedOpts
): Promise<Array<RollupOptions & { output: OutputOptions }>> {
): Promise<RollupOptionsWithOutput[]> {
const allInputs = concatAllArray(
opts.input.map((input: string) =>
createAllFormats(opts, input).map(
(options: TsdxOptions, index: number) => ({
(options: TSDXOptions, index: number) => ({
...options,
// We want to know if this is the first run for each entryfile
// for certain plugins (e.g. css)
Expand All @@ -35,7 +40,7 @@ export async function createBuildConfigs(
);

return await Promise.all(
allInputs.map(async (options: TsdxOptions, index: number) => {
allInputs.map(async (options: TSDXOptions, index: number) => {
// pass the full rollup config to tsdx.config.js override
const config = await createRollupConfig(options, index);
return tsdxConfig.rollup(config, options);
Expand All @@ -46,7 +51,7 @@ export async function createBuildConfigs(
function createAllFormats(
opts: NormalizedOpts,
input: string
): [TsdxOptions, ...TsdxOptions[]] {
): AtLeastOneTSDXOptions {
return [
opts.format.includes('cjs') && {
...opts,
Expand Down Expand Up @@ -85,5 +90,5 @@ function createAllFormats(
env: 'production',
input,
},
].filter(Boolean) as [TsdxOptions, ...TsdxOptions[]];
].filter(Boolean) as AtLeastOneTSDXOptions;
}
14 changes: 7 additions & 7 deletions src/createRollupConfig.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { safeVariableName, safePackageName, external } from './utils';
import { paths } from './constants';
import { RollupOptions } from 'rollup';
import { RollupOptions, Plugin } from 'rollup';
import { terser } from 'rollup-plugin-terser';
import { DEFAULT_EXTENSIONS as DEFAULT_BABEL_EXTENSIONS } from '@babel/core';
import commonjs from '@rollup/plugin-commonjs';
Expand All @@ -14,8 +14,8 @@ import typescript from 'rollup-plugin-typescript2';
import ts from 'typescript';

import { extractErrors } from './errors/extractErrors';
import { babelPluginTsdx } from './babelPluginTsdx';
import { TsdxOptions } from './types';
import { babelPluginTSDX } from './babelPluginTSDX';
import { TSDXOptions } from './types';

const errorCodeOpts = {
errorMapFilePath: paths.appErrorsJson,
Expand All @@ -25,7 +25,7 @@ const errorCodeOpts = {
let shebang: any = {};

export async function createRollupConfig(
opts: TsdxOptions,
opts: TSDXOptions,
outputNum: number
): Promise<RollupOptions> {
const findAndRecordErrorCodes = await extractErrors({
Expand Down Expand Up @@ -71,7 +71,7 @@ export async function createRollupConfig(
// Rollup has treeshaking by default, but we can optimize it further...
treeshake: {
// We assume reading a property of an object never has side-effects.
// This means tsdx WILL remove getters and setters defined directly on objects.
// This means TSDX WILL remove getters and setters defined directly on objects.
// Any getters or setters defined on classes will not be effected.
//
// @example
Expand Down Expand Up @@ -184,7 +184,7 @@ export async function createRollupConfig(
check: !opts.transpileOnly && outputNum === 0,
useTsconfigDeclarationDir: Boolean(tsCompilerOptions?.declarationDir),
}),
babelPluginTsdx({
babelPluginTSDX({
exclude: 'node_modules/**',
extensions: [...DEFAULT_BABEL_EXTENSIONS, 'ts', 'tsx'],
passPerPreset: true,
Expand Down Expand Up @@ -213,6 +213,6 @@ export async function createRollupConfig(
toplevel: opts.format === 'cjs',
warnings: true,
}),
],
].filter(Boolean) as Plugin[],
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a simple one and doesn't change any behavior, but the type-cast seemed to have eroded some typing requirements that exist without it. E.g. Rollup requires each Plugin to have a name property, and some of these custom ones here do not.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ahh think I need a type predicate here per microsoft/TypeScript#20812. I've been using these type-casts after filter like #54 did, but a type predicate / type guard is the right way to do it that I believe will prevent eroding of types

Copy link
Collaborator Author

@agilgur5 agilgur5 Oct 26, 2020

Choose a reason for hiding this comment

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

So I tried the below:

].filter((e): e is Plugin => Boolean(e))

but that didn't fully work either. It also eroded some types, like the name property requirement mentioned above 😕

With the TSDXOptions cast it didn't work because it doesn't seem to be able to infer the AtLeastOne portion 😕 Using enums didn't help with that either 😕
But I was able to fix some type erosion since the esm format one doesn't fit the type since it's missing env. Need to link those two together somehow...

For this Rollup config piece, I was able to rewrite the condition && plugin to ...(condition ? [plugin] : []) which I've done before elsewhere, and then that worked, but it is quite verbose... maybe I can abstract that into a function...

};
}
15 changes: 1 addition & 14 deletions src/env.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,4 @@ declare module 'asyncro'; // doesn't have types (unmerged 2+ year old PR: https:
declare module 'enquirer'; // doesn't have types for Input or Select
declare module 'jpjs'; // doesn't ship types (written in TS though)
declare module 'tiny-glob/sync'; // /sync isn't typed (but maybe we can use async?)

// Patch Babel
// @see line 226 of https://unpkg.com/@babel/[email protected]/lib/index.js
declare module '@babel/core' {
export const DEFAULT_EXTENSIONS: string[];
export function createConfigItem(boop: any[], options: any): any[];
}

// Rollup plugins
declare module 'rollup-plugin-terser';
declare module '@babel/traverse';
declare module '@babel/helper-module-imports';

declare module 'lodash.merge';
declare module '@babel/helper-module-imports'; // doesn't have types
4 changes: 3 additions & 1 deletion src/errors/extractErrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ export async function extractErrors(opts: any) {
}

if (!opts.name || !('name' in opts)) {
throw new Error('Missing options. Ensure you pass --name flag to tsdx');
throw new Error(
'Missing options. Ensure you pass --name flag to `tsdx build`'
);
}

const errorMapFilePath = opts.errorMapFilePath;
Expand Down
25 changes: 8 additions & 17 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,7 @@

import sade from 'sade';
import glob from 'tiny-glob/sync';
import {
rollup,
watch,
RollupOptions,
OutputOptions,
RollupWatchOptions,
WatcherOptions,
} from 'rollup';
import { rollup, watch, RollupWatchOptions, WatcherOptions } from 'rollup';
import asyncro from 'asyncro';
import chalk from 'chalk';
import * as fs from 'fs-extra';
Expand Down Expand Up @@ -40,8 +33,9 @@ import {
PackageJson,
WatchOpts,
BuildOpts,
ModuleFormat,
AtLeastOneModuleFormat,
NormalizedOpts,
RollupOptionsWithOutput,
} from './types';
import { createProgressEstimator } from './createProgressEstimator';
import { templates } from './templates';
Expand Down Expand Up @@ -393,13 +387,10 @@ prog
}
try {
const promise = asyncro
.map(
buildConfigs,
async (inputOptions: RollupOptions & { output: OutputOptions }) => {
let bundle = await rollup(inputOptions);
await bundle.write(inputOptions.output);
}
)
.map(buildConfigs, async (inputOptions: RollupOptionsWithOutput) => {
let bundle = await rollup(inputOptions);
await bundle.write(inputOptions.output);
})
.catch((e: any) => {
throw e;
})
Expand All @@ -424,7 +415,7 @@ async function normalizeOpts(opts: WatchOpts): Promise<NormalizedOpts> {
return 'esm';
}
return format;
}) as [ModuleFormat, ...ModuleFormat[]],
}) as AtLeastOneModuleFormat,
};
}

Expand Down
14 changes: 12 additions & 2 deletions src/types.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { RollupOptions, OutputOptions } from 'rollup';

interface SharedOpts {
// JS target
target: 'node' | 'browser';
Expand All @@ -8,6 +10,7 @@ interface SharedOpts {
}

export type ModuleFormat = 'cjs' | 'umd' | 'esm' | 'system';
export type AtLeastOneModuleFormat = [ModuleFormat, ...ModuleFormat[]];

export interface BuildOpts extends SharedOpts {
name?: string;
Expand All @@ -29,10 +32,10 @@ export interface NormalizedOpts
extends Omit<WatchOpts, 'name' | 'input' | 'format'> {
name: string;
input: string[];
format: [ModuleFormat, ...ModuleFormat[]];
format: AtLeastOneModuleFormat;
}

export interface TsdxOptions extends SharedOpts {
export interface TSDXOptions extends SharedOpts {
// Name of package
name: string;
// path to file
Expand All @@ -48,6 +51,7 @@ export interface TsdxOptions extends SharedOpts {
// Only transpile, do not type check (makes compilation faster)
transpileOnly?: boolean;
}
export type AtLeastOneTSDXOptions = [TSDXOptions, ...TSDXOptions[]];

export interface PackageJson {
name: string;
Expand All @@ -60,3 +64,9 @@ export interface PackageJson {
node?: string;
};
}

export type RollupOptionsWithOutput = RollupOptions & { output: OutputOptions };

export interface TSDXConfig {
rollup: (config: RollupOptions, options: TSDXOptions) => RollupOptions;
}
Comment on lines +70 to +72
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This still has the same problem as in #823 (comment) . Should probably try being more specific here and in createRollupConfig.ts.

2 changes: 1 addition & 1 deletion templates/basic/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ Two actions are added by default:

## Optimizations

Please see the main `tsdx` [optimizations docs](https://github.com/palmerhq/tsdx#optimizations). In particular, know that you can take advantage of development-only optimizations:
Please see the main TSDX [optimizations docs](https://github.com/palmerhq/tsdx#optimizations). In particular, know that you can take advantage of development-only optimizations:

```js
// ./types/index.d.ts
Expand Down
2 changes: 1 addition & 1 deletion templates/react-with-storybook/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ Two actions are added by default:

## Optimizations

Please see the main `tsdx` [optimizations docs](https://github.com/palmerhq/tsdx#optimizations). In particular, know that you can take advantage of development-only optimizations:
Please see the main TSDX [optimizations docs](https://github.com/palmerhq/tsdx#optimizations). In particular, know that you can take advantage of development-only optimizations:

```js
// ./types/index.d.ts
Expand Down
2 changes: 1 addition & 1 deletion templates/react/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ Two actions are added by default:

## Optimizations

Please see the main `tsdx` [optimizations docs](https://github.com/palmerhq/tsdx#optimizations). In particular, know that you can take advantage of development-only optimizations:
Please see the main TSDX [optimizations docs](https://github.com/palmerhq/tsdx#optimizations). In particular, know that you can take advantage of development-only optimizations:

```js
// ./types/index.d.ts
Expand Down
4 changes: 2 additions & 2 deletions website/pages/customization.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ module.exports = {
The `options` object contains the following:

```tsx
export interface TsdxOptions {
export interface TSDXOptions {
// path to file
input: string;
// Name of package
Expand Down Expand Up @@ -73,7 +73,7 @@ module.exports = {

## Babel

You can add your own `.babelrc` to the root of your project and TSDX will **merge** it with [its own Babel transforms](./src/babelPluginTsdx.ts) (which are mostly for optimization), putting any new presets and plugins at the end of its list.
You can add your own `.babelrc` to the root of your project and TSDX will **merge** it with [its own Babel transforms](./src/babelPluginTSDX.ts) (which are mostly for optimization), putting any new presets and plugins at the end of its list.

## Jest

Expand Down
Loading