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 tree shaking #415

Merged
merged 6 commits into from
Jun 21, 2022
Merged

Fix tree shaking #415

merged 6 commits into from
Jun 21, 2022

Conversation

0x009922
Copy link
Contributor

@0x009922 0x009922 commented May 26, 2022

Changes

  • Remove a lot of unnecessary SFC names declaration
  • ui library now has unit & after-build tests driven by vitest
  • After-build tests ensure tree-shakeability, e.g. library does not have any side-effects.

For other details please see changesets.

Explorations

  • TypeScript enums are evil. They are not tree-shakeable because they are compiled to IIFE:

    enum Status {
      Info
    }
    var Status;
    (function (Status) {
      Status[Status["Info"] = 0] = "Info";
    })(Status || (Status = {}));

    The way I used to work-around it:

    export const Status = {
      Info: 'info',
      Success: 'success',
      Warning: 'warning',
      Error: 'error',
    } as const
    
    export type Status = typeof Status[keyof typeof Status]
    
    function acceptAnyStatus(status: Status) {}
    
    acceptAnyStatus(Status.Error)
    
    function acceptInfoOnly(info: typeof Status.Info) {}
    
    acceptInfoOnly(Status.Info)
  • jsoneditor and lodash (**edit:**not sure) are evil. They have side-effects. Fortunately, they are only used within SJsonInput. I excluded this component from the bundle temporarily - anyway it is unused and out of design system.

  • body-scroll-lock is evil too, but only a bit. It has a small side-effect check code. I think it can be implemented in pure manner, and maybe I'll open a PR. upd: The package is not tree-shakeable due to side-effects willmcpo/body-scroll-lock#254 and Fix tree shake by adding some /* @__PURE__ */ marks willmcpo/body-scroll-lock#255
    For now, I exclude it from SModal component, but I add a new component - SBodyScrollLockProvider. SModal will use it's locker by default. For details please see sources and SModal.spec.cy for usage example (it uses body-scroll-lock small wrap).

  • defineComponent() should always be used with /* @__PURE__ */ or /* #__PURE__ */ prefix. Generally, it should be avoided.

  • vite.config.tsbuild.target set to esnext. It produces more pure code, and in some cases it matters for tree-shaking.
    The exact case is the following:

    • SFC with 2 <script lang="ts"> blocks;

    • Some component options:

      export default {
        inheritAttrs: false
      }

    As a result, without esnext target there is a dirt that prevents component to be tree-shaken.

@0x009922 0x009922 added the next Related to next lib iteration, i.e. based on Vue 3 label May 26, 2022
@0x009922 0x009922 linked an issue May 26, 2022 that may be closed by this pull request
Comment on lines 15 to 19
console.warn(
`[soramitsu-ui warn]: Invalid prop: type check failed for prop "${name}". Expected: ${formattedList}, got ${formattedValue}`,
`[soramitsu-ui warn]: Invalid prop: type check failed for prop "${String(
name,
)}". Expected: ${formattedList}, got ${formattedValue}`,
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

name var could be a Symbol. TypeScript prevented runtime error!


"types": ["vitest/importMeta"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make in-source tests possible.

Comment on lines +37 to +39
define: {
'import.meta.vitest': 'undefined',
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will be useful when there some in-source tests appear.

Copy link

@Ctepan Ctepan left a comment

Choose a reason for hiding this comment

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

There are new components, so it's necessary to make same operations to them

@0x009922 0x009922 merged commit be51e0e into next Jun 21, 2022
@0x009922 0x009922 deleted the fix-tree-shaking branch June 21, 2022 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next Related to next lib iteration, i.e. based on Vue 3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix tree-shaking
3 participants