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

Turboo and esbuild #161

Merged
merged 13 commits into from Nov 11, 2022
Merged

Turboo and esbuild #161

merged 13 commits into from Nov 11, 2022

Conversation

wootsbot
Copy link
Member

@wootsbot wootsbot commented Oct 25, 2022

close #154

@wootsbot
Copy link
Member Author

@LitoMore can this resolve the build speed, or do you have a better setup?

@LitoMore
Copy link
Member

Yes, it's much faster. We can also optimize the declaration file.

We could define the component type in a types.ts file. Then import the component type from this file.

// types.ts
export type IconProps = React.ComponentPropsWithoutRef<'svg'> & {
  /**
   * Hex color or color name
   */
  title?: string;
  /**
   * The size of the Icon.
   */
  color?: string;
  /**
   * The title provides an accessible short text description to the SVG
   */
  size?: string | number;
};
// components/XXX.tsx
import {IconProps} from '../types.ts';

const XXX = React.forwardRef<SVGSVGElement, Icon>(function XXX({color = 'currentColor', size = 24, title = "xxx", ...others}, ref) {

  return (
    <svg xmlns="http://www.w3.org/2000/svg" width={size} height={size} fill={color} viewBox="0 0 24 24" ref={ref} {...others}>
      <title>{title}</title>
      <path d="foo bar" />
    </svg>
  );
});

export default XXX;

This might reduce our file size.

The React.forwardRef<SVGSVGElement, Icon>(...) {} part can be optimized in the same way.

@wootsbot
Copy link
Member Author

wootsbot commented Oct 26, 2022

@LitoMore good idea, the changes are applied...

@LitoMore
Copy link
Member

LitoMore commented Oct 27, 2022

// base.tsx
import * as React from 'react';
import { IconProps } from './types';

const baseIcon = (iconTitle: string, iconPath: string) => React.forwardRef<SVGSVGElement, IconProps>((function ({title = iconTitle, color = 'currentColor', size = 24, ...others}, ref) {
  return (
    <svg xmlns="http://www.w3.org/2000/svg" width={size} height={size} fill={color} viewBox="0 0 24 24" ref={ref} {...others}>
      <title>{title}</title>
      <path d={iconPath} />
    </svg>
  )
}));

export default baseIcon;
// components/XXX.ts
import baseIcon from './base';

const XXX = baseIcon("xxx", "foo bar");
export default XXX;
// The template in generate-components
import baseIcon from '../base';

const ${componentName} = baseIcon('${baseName}', '${SimpleIcons[baseName].path}');

export default ${componentName};

@wootsbot
Copy link
Member Author

// base.tsx
import * as React from 'react';
import { IconProps } from './types';

const baseIcon = (iconTitle: string, iconPath: string) => React.forwardRef<SVGSVGElement, IconProps>((function ({title = iconTitle, color = 'currentColor', size = 24, ...others}, ref) {
  return (
    <svg xmlns="http://www.w3.org/2000/svg" width={size} height={size} fill={color} viewBox="0 0 24 24" ref={ref} {...others}>
      <title>{title}</title>
      <path d={iconPath} />
    </svg>
  )
}));

export default baseIcon;
// components/XXX.ts
import baseIcon from './base';

const XXX = baseIcon("xxx", "foo bar");
export default XXX;
// The template in generate-components
import baseIcon from '../base';

const ${componentName} = baseIcon('${baseName}', '${SimpleIcons[baseName].path}');

export default ${componentName};

ready....

@LitoMore
Copy link
Member

LitoMore commented Oct 28, 2022

I optimized the declaration file with https://github.com/icons-pack/react-simple-icons/commits/4ba02e02db28ab6e824a15a653a6dba0a2f045d7.

Before After

Even more, we could remove those empty lines before icon decorations. But I didn't find any documentation for this.

// Before
declare const FortyTwo: IconType;

declare const Dotenv: IconType;

declare const Dotnet: IconType;
// After
declare const FortyTwo: IconType;
declare const Dotenv: IconType;
declare const Dotnet: IconType;

Another point, those exports at the bottom of the file can be optimized as well:

-declare const DotNet: IconType;
+export const DotNet: IconType;
-export {DotNet};

We could disable the dts in tsup and generate the index.d.ts file with our script.

What do you think?

@LitoMore
Copy link
Member

We can also optimize the file writing. Just change multiple writes to one write for index.ts.

For those components files, we could use fs/promises with Promise.all to generate files.

We can do this enhancement in another PR.

@wootsbot
Copy link
Member Author

I optimized the declaration file with https://github.com/icons-pack/react-simple-icons/commits/4ba02e02db28ab6e824a15a653a6dba0a2f045d7.

Before After

Even more, we could remove those empty lines before icon decorations. But I didn't find any documentation for this.

// Before
declare const FortyTwo: IconType;

declare const Dotenv: IconType;

declare const Dotnet: IconType;
// After
declare const FortyTwo: IconType;
declare const Dotenv: IconType;
declare const Dotnet: IconType;

Another point, those exports at the bottom of the file can be optimized as well:

-declare const DotNet: IconType;
+export const DotNet: IconType;
-export {DotNet};

We could disable the dts in tsup and generate the index.d.ts file with our script.

What do you think?

We could disable the dts in tsup and generate the index.d.ts file with our script.

What do you think?

For me it's pretty good ahead with the change.

@wei wei mentioned this pull request Nov 10, 2022
@LitoMore
Copy link
Member

LitoMore commented Nov 10, 2022

@wootsbot Optimization works done. Could you help to review those changes?

I optimized the build time from 11s to 4s.

Before After

@wootsbot
Copy link
Member Author

@LitoMore I think we can continue to merge these changes.

@wootsbot wootsbot merged commit 282b51c into main Nov 11, 2022
@wootsbot wootsbot deleted the turboo-and-esbuild branch November 11, 2022 00:43
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.

Move to esbuild
2 participants