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

perf: mark functions as @__NO_SIDE_EFFECTS__ #995

Merged
merged 4 commits into from
Dec 26, 2024

Conversation

antfu
Copy link
Contributor

@antfu antfu commented Dec 24, 2024

This PR adds @__NO_SIDE_EFFECTS__ notation for all the schema functions:
https://github.com/javascript-compiler-hints/compiler-notations-spec/blob/main/no-side-effects-notation-spec.md

This would improve tree-shaking of Valibot on users side. For example, the snippet from the docs:

import * as v from 'valibot'

const LoginSchema = v.object({
  email: v.pipe(v.string(), v.email()),
  password: v.pipe(v.string(), v.minLength(8)),
})

Because the object() can not be treated as side-effects free, even LoginSchema is not used, the final bundle still contains an orphan object() call with it's all dependencies (bundle ~3KB).

One solution is to add /* @__PURE__ */ on the user side,

import * as v from 'valibot'

const LoginSchema = /* @__PURE__ */ v.object({
  email: v.pipe(v.string(), v.email()),
  password: v.pipe(v.string(), v.minLength(8)),
})

The downside is this would require the user's manual effort on every schema.

With @__NO_SIDE_EFFECTS__, it will give a hint to the bundler to know object() is side-effects free, and eventually make the whole file shakable (bundle 0KB)

Example of bundling using Rollup: https://stackblitz.com/edit/node-5rxpc1jl

This PR is made from the discussion with @serkodev

@fabian-hiller
Copy link
Owner

Thank you Anthony and SerKo. What bundlers support this comment? Only Rollup? Shouldn't LoginSchema be removed if it's not used? One use case where this is probably useful is when a schema is only used for the TS type, but not for runtime validation.

Hint: My tests with /* @__PURE__ */ in the provided StackBlitz did not work. I used the following code:

import * as v from 'valibot'

const LoginSchema = /* @__PURE__ */ v.object({
  email: v.pipe(v.string(), v.email()),
  password: v.pipe(v.string(), v.minLength(8)),
})

@fabian-hiller fabian-hiller self-assigned this Dec 24, 2024
@fabian-hiller fabian-hiller added the enhancement New feature or request label Dec 24, 2024
@serkodev
Copy link
Contributor

The /* @__PURE__ */ annotation is supported by most bundlers. You can refer to the compatibility details in the #__PURE__ and #__NO_SIDE_EFFECTS__ documentation.

Sorry for the misleading. In the test code, the /* @__PURE__ */ annotation preceding v.object indicates that the function call to v.object has no side effects. However, in this specific case, v.pipe does introduce side effects.

Here is the correct version, it will be tree-shaken successfully:

import * as v from 'valibot';

const LoginSchema = v.object({
  email: /* @__PURE__ */ v.pipe(v.string(), v.email()),
  password: /* @__PURE__ */ v.pipe(v.string(), v.minLength(8)),
});

@antfu
Copy link
Contributor Author

antfu commented Dec 25, 2024

Yeah, theoretically, you need mark every call to be pure to ensure it's complete side-effects free:

import * as v from 'valibot';

const LoginSchema = /* @__PURE__ */ v.object({
  email: /* @__PURE__ */ v.pipe(/* @__PURE__ */ v.string(), /* @__PURE__ */ v.email()),
  password: /* @__PURE__ */ v.pipe(/* @__PURE__ */ v.string(), /* @__PURE__ */ v.minLength(8)),
});

It's just that Rollup is trying to be smart and looking into the implementation to infer that string() and email() are pure. But it fails to treat object() pipe() etc pure due to the circular reference inside. It's very hard to end users to know which functions are pure and which are not - it's also not safe as a new version that changes the implementation might end up with unexpected bundle result.

@__NO_SIDE_EFFECTS__ is designed to solve that by moving the notation from end-user side to library authors.

@fabian-hiller
Copy link
Owner

Thank you!

@fabian-hiller fabian-hiller merged commit e86f355 into fabian-hiller:main Dec 26, 2024
@antfu antfu deleted the feat/mark-no-side-effects branch December 31, 2024 05:16
@fabian-hiller
Copy link
Owner

v1.0.0-beta.10 is available

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants