text:
Supporte Metadata like description

Discussed in https://github.com/fabian-hiller/valibot/discussions/368

Originally posted by xcfox January 14, 2024
Valibot is a super cool Schema Builder! I am attracted by Valibot's modular design.
I plan to use Valibot as an Entity Schema Builder for MikroORM.
So far, I am using TypeScript's class and Decorators to define MikroORM's Entity like this:
@Entity()
export class BaseEntity {
  @PrimaryKey()
  id!: number

  @Property()
  createdAt = new Date()

  @Property({ onUpdate: () => new Date() })
  updatedAt = new Date()
}

interface WithName {
  name: string
}

@Entity()
export class User extends BaseEntity implements WithName {
  @Property({ unique: true, comment: "user's name" })
  name!: string

  @index()
  @Property({ nullable: true })
  email: string

  @Property({ type: 'text' })
  password!: string
}

Because TypeScript's class doesn't support multiple inheritance, I have to define the name field repeatedly.
Maybe I can use Valibot and custom pipelines to solve this problem:
import { intersect, Output, nullable, date, string, object } from 'valibot'
import { primaryKey, property, index, unique, description } from './custom'

const baseEntity = object({
  id: string([primaryKey()]),
  createdAt: date([property()]),
  updatedAt: date([property()]),
})

const withName = object({
  name: string([property(), index(), description("user's name")]),
})

export const userEntity = intersect([
  object({
    email: nullable(string([property(), unique()])),
    password: string([property()]),
  }),
  baseEntity,
  withName,
])

export interface IUser extends Output<typeof userEntity> {}

Here I use many custom pipelines: primaryKey, property, index, unique, description. However, the current version of Valibot's pipeline only supports Validation and Transformation. I don't think these custom pipes belong to either Validation or Transformation.
Once Metadata is supported, Valibot has the ability to shine in all scenarios where a Schema needs to be defined, such as graphql, typegoose, typeorm and not just for data validation like zod. Valibot's modularity, combined with Metadata, makes for a very powerful Schema Builder! All projects using TypeScript can use Valibot to define Schemas, and the development experience will be far superior to that of class and Decorators.

I have investigated this use case. As far as I understand my code, the current implementation of Valibot does not limit this functionality. This feature can already be added by third party libraries by providing pipeline functions like index and description. Here is an example that implements index and description:
import * as v from 'valibot';

/**
 * Index metadata type.
 */
type IndexMetadata<TInput> = {
  async: false;
  _parse(input: TInput): v.PipeActionResult<TInput>;
  type: 'index';
};

/**
 * Creates a pipeline metadata action to indicate that the field is an index.
 *
 * @returns A metadata action.
 */
function index<TInput>(): IndexMetadata<TInput> {
  return {
    type: 'index',
    async: false,
    _parse(input) {
      return v.actionOutput(input);
    },
  };
}

/**
 * Description metadata type.
 */
type DescriptionMetadata<TInput> = {
  async: false;
  _parse(input: TInput): v.PipeActionResult<TInput>;
  type: 'description';
  description: string;
};

/**
 * Creates a pipeline metadata action that adds a description the field.
 *
 * @param description The description to be added.
 *
 * @returns A metadata action.
 */
function description<TInput>(description: string): DescriptionMetadata<TInput> {
  return {
    type: 'description',
    description,
    async: false,
    _parse(input) {
      return v.actionOutput(input);
    },
  };
}

// Create user entity schema
const UserEntitySchema = v.object({
  name: v.string([
    index(),
    description('The name of the user.'),
    v.maxLength(30),
  ]),
  // ...
});

// Read index metadata
const shouldBeIndexed = !!UserEntitySchema.entries.name.pipe?.find(
  (action) => 'type' in action && action.type === 'index'
);

The more important thing for us as a community is to decide if this functionality belongs in the scope of Valibot, and if the library should get a first-class metadata API. If we decide to do so, we should discuss if we want to add this to our current pipeline feature as shown above, if we want to rename and reimplement the pipeline feature so that it is not necessary to include async and _parse for metadata objects, or if we want to implement this completely different, for example with a metadata method that adds the metadata directly to the schema object apart from the validation and transformation actions.
I look forward to hearing what you think.

@fabian-hiller  You're absolutely right.
Over the past few days, I've built the valibot-mikro library for building mikro entities with the help of valibot.
What I'm doing now is disguising metadata like property(), index() as BaseValidation.
/**
 * Mikro property meta.
 */
export type PropertyMeta<Entity = any, TInput = any> = BaseValidation<TInput> & {
	/**
	 * The validation type.
	 */
	type: "mikro_property"

	/**
	 * The property meta.
	 */
	meta?: Partial<EntitySchemaProperty<TInput, Entity>>
}

This works fine, but it feels a little twisted.
As you say, this contains unnecessary async and _parse.
I strongly agree with rename and reimplement the pipeline feature to support pure metadata.
In addition, actually, in valibot-mikro, I've implemented some methods similar to the metadata method, that is, manyToMany, manyToOne, which are actually relation types specific to MikroORM.
In my practice, I realized that there should be no need for an abstract metadata method method, but many more explicit metadata methods, such as property, index, columnName.
By putting the metadata in the pipeline, we can clearly identify the type of the field, while the secondary metadata is in the secondary position:
const User = object({
  name: number([property(), index(), columnName('a_strange_column_name')])
})

But by using metadata as a wrapper, the code starts to get confused, metadata() is not a type, but it is in the place of the type:
const User = object({
  name: metadata(number(), [property(), index(), columnName('a_strange_column_name')])
})

Such an approach may could be even more messy:
const User = object({
  name: columnName(property(index(number())), 'a_strange_column_name')
})

As a community, could we also consider building in some generic metadata like description() so that when writing the schema, we don't have to write generic meta information for every 3rd party library. I see that the describe() method exists in zod.

I also noticed that not every schema can accept pipe arguments, such as recursive(), enum_(), picklist().
I understand that these schemas include validation by themselves and may not need an additional pipe, but when I use valibot as a schema builder, I do need to provide a way for all schemas to pass in a metadata.
Do you use a plan to complement the pipe parameter for all schemas?
Or can we add additional meta attributes to the schema? For example, for BaseSchema:
/**
 * Base schema type.
 */
export type BaseSchema<TInput = any, TOutput = TInput> = {
  /**
   * store the metadata.
   */
  meta?: BaseMetadata[];
  /**
   * Whether it's async.
   */
  async: false;
  /**
   * Parses unknown input based on its schema.
   *
   * @param input The input to be parsed.
   * @param info The parse info.
   *
   * @returns The parse result.
   *
   * @internal
   */
  _parse(input: unknown, info?: ParseInfo): SchemaResult<TOutput>;
  /**
   * Input and output type.
   *
   * @internal
   */
  _types?: { input: TInput; output: TOutput };
};

The advantage of this is that the metadate can be completely ignored when running validation for better performance.

I like the idea of putting the metadata under its own key. However, this would require that we put the metadata in a different array to separate from the pipe, or we would have to filter and process the pipeline when creating the schema to extract the metadata. Also, I think it would be nice to access the metadata directly via .metadata?.description. Instead of writing .metadata?.find((item) => item.type === 'description').
const User = object({
  // Note: This would require us to always specify the pipeline, even if it is empty
  name: string([], [index(), description('Lorem ipsum')]),
  age: number([minValue(10)], [index(), description('Lorem ipsum')]),
})

Another idea is to provide the metadata directly as an object instead of an array of functions. I suspect this will also result in the smallest bundle size.
const User = object({
  name: string([], { index: true, description: 'Lorem ipsum' }),
  age: number([minValue(10)], { index: true, description: 'Lorem ipsum' }),
})

I totally agree with the use of key-value pairs to store metadata.
For the best development experience, I think we should put the metadata in the same array as the validators and transformations:
const User = object({
  name: string([index(), description('Lorem ipsum')]),
  age: number([minValue(10), index(), description('Lorem ipsum')]),
})

To do this we need to do some extra work when creating the schema, which fortunately is fairly simple.
Also, I noticed that ErrorMessage should actually be treated as metadata, so that we don't have to leave a separate pass position for message, and for all schemas we can use rest parameters to pass the pipeline so that we don't have to explicitly build arrays:
const User = object({
  name: string(message("The name is illegal"), index(), description('Lorem ipsum')),
  age: number(minValue(10), index(), description('Lorem ipsum')),
})

This will result in a huge break change.
Another idea is to use the current location of message to accept metadata, which should accept a string or metadata object for compatibility with older code:
const User = object({
  name: string({ description:"Lorem ipsum", index: true }),
  age: number({ message:"too young", description:"Lorem ipsum", index: true },[minValue(10)]),
})

Taking into account compatibility, development experience and packaging size, I think this is a very good solution. We also need to expose the Metadata interface externally for code hinting and type checking:
// valibot
export interface Metadata {
  description?: string
  message?: string
}

Users need to add additional global type declarations when installing third-party libraries:
// env.d.ts
import { Metadata } from 'valibot'
import { MikroMetadata } from 'valibot-mikro'
declare module 'valibot' {
  export interface Metadata extends MikroMetadata {}
}

Thanks again for the details! I will look into this and get back to you with the results.

For the best development experience, I think we should put the metadata in the same array as the validators and transformations

I agree that the DX is very nice this way. The downside for me is the internal implementation because it requires us to filter and process the pipeline on schema creation. This would increase the bundle size and slow down the startup performance. Since I expect that the metadata feature will only be used by a small fraction of users, I am not sure if I want to go this way.

Also, I noticed that ErrorMessage should actually be treated as metadata

I see your point. On the other hand, ErrorMessage is used as the first optional argument of every schema and validation functions and validation functions have no pipeline. The current approach makes it consistent across the library.

and for all schemas we can use rest parameters to pass the pipeline so that we don't have to explicitly build arrays

When I first implemented Valibot in July 2023, I considered this approach. In the end I decided against it because it makes it much harder to distinguish the pipe arguments and limits the API for complex schema functions like object that have other optional arguments. Also, the formatting with Prettier looks a lot more chaotic this way.

Another idea is to use the current location of message to accept metadata

This might work, and I will consider it. It might make it harder to handle and process custom error messages, which is an important feature of the library. Also, I am not sure if message should be part of the metadata.

We also need to expose the Metadata interface externally for code hinting and type checking

Great idea! I agree!

Although I have argued against your suggestions, this comment is not a final decision, and I welcome your feedback. In the end, my goal is to work with the community to create a great schema library.
If we decide to add the metadata object as the last optional argument of each schema function, I see several benefits.

The current API remains unchanged, there are no breaking changes
The optional metadata argument can be easily identified in defaultArgs
The metadata argument can be added to the schema object without processing
The bundle size and performance footprint is minimal

const User = object({
  name: string({ index: true, description: 'Lorem ipsum' }),
  age: number([minValue(10)], { index: true, description: 'Lorem ipsum' }),
})

I strongly agree with adding the metadata object as the last optional argument of each schema function.
This approach is not much different on DX compared to using the current location of message to accept metadata.
The only problem is that this leads to more verbose signatures for schema functions, such as for object() functions:

Click to show code
/**
 * Creates an object schema.
 *
 * @param entries The object entries.
 * @param metadata The schema metadata.
 *
 * @returns An object schema.
 */
export function object<TEntries extends ObjectEntries>(
  entries: TEntries,
  metadata?: Metadata
): ObjectSchema<TEntries>;

/**
 * Creates an object schema.
 *
 * @param entries The object entries.
 * @param pipe A validation and transformation pipe.
 * @param metadata The schema metadata.
 *
 * @returns An object schema.
 */
export function object<TEntries extends ObjectEntries>(
  entries: TEntries,
  pipe?: Pipe<ObjectOutput<TEntries, undefined>>,
  metadata?: Metadata
): ObjectSchema<TEntries>;

/**
 * Creates an object schema.
 *
 * @param entries The object entries.
 * @param message The error message.
 * @param pipe A validation and transformation pipe.
 * @param metadata The schema metadata.
 *
 * @returns An object schema.
 */
export function object<TEntries extends ObjectEntries>(
  entries: TEntries,
  message?: ErrorMessage,
  pipe?: Pipe<ObjectOutput<TEntries, undefined>>,
  metadata?: Metadata
): ObjectSchema<TEntries>;

/**
 * Creates an object schema.
 *
 * @param entries The object entries.
 * @param rest The object rest.
 * @param pipe A validation and transformation pipe.
 * @param metadata The schema metadata.
 *
 * @returns An object schema.
 */
export function object<
  TEntries extends ObjectEntries,
  TRest extends BaseSchema | undefined
>(
  entries: TEntries,
  rest: TRest,
  pipe?: Pipe<ObjectOutput<TEntries, TRest>>,
  metadata?: Metadata
): ObjectSchema<TEntries, TRest>;

/**
 * Creates an object schema.
 *
 * @param entries The object entries.
 * @param rest The object rest.
 * @param message The error message.
 * @param pipe A validation and transformation pipe.
 * @param metadata The schema metadata.
 *
 * @returns An object schema.
 */
export function object<
  TEntries extends ObjectEntries,
  TRest extends BaseSchema | undefined
>(
  entries: TEntries,
  rest: TRest,
  message?: ErrorMessage,
  pipe?: Pipe<ObjectOutput<TEntries, TRest>>,
  metadata?: Metadata
): ObjectSchema<TEntries, TRest>;

export function object<
  TEntries extends ObjectEntries,
  TRest extends BaseSchema | undefined = undefined
>(
  entries: TEntries,
  arg2?: Metadata | Pipe<ObjectOutput<TEntries, TRest>> | ErrorMessage | TRest,
  arg3?: Metadata | Pipe<ObjectOutput<TEntries, TRest>> | ErrorMessage,
  arg4?: Metadata | Pipe<ObjectOutput<TEntries, TRest>>,
  arg5?: Metadata
): ObjectSchema<TEntries, TRest>

This looks like it will come with more maintenance costs, other than that the solution is perfect!

Which metadata properties should Valibot ship by default? Can you create a list for me?

We should refer to existing popular standards such as annotations for json-schema, parameter for openapi, GraphQL or ts-doc.
I researched the above specification and came up with some generic metadata fields:
/**
 * Schema metadata type.
 */
export interface SchemaMetadata<T = any> {
  /**
   * The name of the schema.
   */
  name?: string;
  /**
   * A brief description of the schema.
   */
  description?: string;
  /**
   *  The instance value of the schema should not be used and the schema may be removed in the future.
   */
  deprecated?: boolean;

  /**
   * The `examples` is a place to provide an array of examples that validate against the schema.
   */
  examples?: T[];
}

Of course, we can also strictly follow the json-schema specification.
Then we will have the following build-in metadata fields:

/**
 * Schema metadata type.
 */
export interface SchemaMetadata<T = any> {
  /**
   * The title of the schema.
   */
  title?: string;
  /**
   * A brief description of the schema.
   */
  description?: string;

  /**
   * The `examples` is a place to provide an array of examples that validate against the schema.
   */
  examples?: T[];

  /**
   * The `readOnly` keyword indicates that the value of the instance is managed exclusively by the owning authority, and attempts by an application to modify the value of this property are expected to be ignored or rejected by that owning authority.
   */
  readOnly?: boolean;

  /**
   * The `writeOnly` keyword indicates that the value is never present when the instance is retrieved from the owning authority.
   */
  writeOnly?: boolean;

  /**
   *  The instance value of the schema should not be used and the schema may be removed in the future.
   */
  deprecated?: boolean;
}

What about SQL properties like index, unique, primary key?

I don't think SQL properties should be included in the valibot package itself. This is because when dealing with a specific business, the situation is complex and varied.
In the TypeScript world, each ORM has its own unique way of defining schema:
https://sequelize.readthedocs.io/
https://orm.drizzle.team/docs/sql-schema-declaration
https://typeorm.io/entities#what-is-entity
https://typegoose.github.io/typegoose/docs/guides/quick-start-guide/#quick-overview-of-typegoose
It is quite difficult to adapt valibot to such a variety of situations or to follow ORM package updates.
In order for valibot to be a generalized schema builder, I think it needs to be used to create various community packages such as valibot-drizzle, valibot-typeorm.
And valibot itself should remain modular and extensible so that developers can easily add unique metadata:
// env.d.ts
import { SchemaMetadata } from 'valibot'
import { MikroMetadata } from 'valibot-mikro'
declare module 'valibot' {
  export interface SchemaMetadata extends MikroMetadata {}
}

I'm trying to implement this feature, and the current solution is to use metadata as the last parameter of the schema function.
But now I'm facing a tricky problem. TypeScript doesn't seem to hit the overload correctly.

Here is the array() declaration
/**
 * Creates a array schema.
 *
 * @param item The item schema.
 * @param pipe A validation and transformation pipe.
 * @param metadata The schema metadata.
 *
 * @returns A array schema.
 */
export function array<TItem extends BaseSchema>(
  item: TItem,
  pipe?: Pipe<Output<TItem>[]>,
  metadata?: SchemaMetadata<Input<TItem>>
): ArraySchema<TItem>;

/**
 * Creates a array schema.
 *
 * @param item The item schema.
 * @param message The error message.
 * @param pipe A validation and transformation pipe.
 * @param metadata The schema metadata.
 *
 * @returns A array schema.
 */
export function array<TItem extends BaseSchema>(
  item: TItem,
  message?: ErrorMessage,
  pipe?: Pipe<Output<TItem>[]>,
  metadata?: SchemaMetadata<Input<TItem>>
): ArraySchema<TItem>;

/**
 * Creates a array schema.
 *
 * @param item The item schema.
 * @param metadata The schema metadata.
 *
 * @returns A array schema.
 */
export function array<TItem extends BaseSchema>(
  item: TItem,
  metadata?: SchemaMetadata<Input<TItem>>
): ArraySchema<TItem>;

export function array<TItem extends BaseSchema>(
  item: TItem,
  arg2?: SchemaMetadata<Input<TItem>> | Pipe<Output<TItem>[]> | ErrorMessage,
  arg3?: SchemaMetadata<Input<TItem>> | Pipe<Output<TItem>[]>,
  arg4?: SchemaMetadata<Input<TItem>>
): ArraySchema<TItem>

const schema2 = array(number(), 'Error', [length(1), includes(123)]);

For the above code, TypeScript, gives the following error message:
No overload matches this call.
  Overload 1 of 3, '(item: NumberSchema<number>, pipe?: Pipe<number[]> | undefined, metadata?: SchemaMetadata<number> | undefined): ArraySchema<NumberSchema<number>, number[]>', gave the following error.
    Argument of type 'string' is not assignable to parameter of type '(BaseValidation<number[]> | BaseTransformation<number[]>)[]'.ts(2769)

But the following code works fine:
const schema2 = array(number(), 'Error', [length(1), includes(123)], {});
const schema3 = array(number(), 'Error', [length(1), includes(123)], undefined);

If you have a spare moment, check out my code implementation here
To avoid the above, I think we should use the current ErrorMessage location for the metadata. This location should accept both ErrorMessage and SchemaMetadata, so that break change can be avoided.
We also reserve a message field in metadata for cases where both ErrorMessage and SchemaMetadata are needed. We can either extract the message in defaultArgs(), or we can just make the message part of the metadata, either way it's easy.
This approach also makes our schema function much simpler:

Here we only need two overloads
/**
 * Creates a array schema.
 *
 * @param item The item schema.
 * @param pipe A validation and transformation pipe.
 *
 * @returns A array schema.
 */
export function array<TItem extends BaseSchema>(
  item: TItem,
  pipe?: Pipe<Output<TItem>[]>
): ArraySchema<TItem>;

/**
 * Creates a array schema.
 *
 * @param item The item schema.
 * @param metadata The schema metadata or error message.
 * @param pipe A validation and transformation pipe.
 *
 * @returns A array schema.
 */
export function array<TItem extends BaseSchema>(
  item: TItem,
  metadata?: ErrorMessage | SchemaMetadata<Input<TItem>>,
  pipe?: Pipe<Output<TItem>[]>
): ArraySchema<TItem>;

export function array<TItem extends BaseSchema>(
  item: TItem,
  arg2?: SchemaMetadata<Input<TItem>> | ErrorMessage | Pipe<Output<TItem>[]>,
  arg3?: Pipe<Output<TItem>[]>
): ArraySchema<TItem>

const schema2 = array(number(), 'Error', [length(1), includes(123)]);
const schema3 = array(number(), { message: 'Error', description: "Lorem ipsum" }, [length(1), includes(123)]);

Let's compare the scenarios in detail: that is, placing metadata at the last parameter and placing metadata at the ErrorMessage position:

point
at the last parameter
at the ErrorMessage position

no breaking changes
⚠ ts may miss the expected function overload
✅

easily identified in defaultArgs
✅
✅

can be added to the schema object without processing
✅
⚠ Requires additional operations on the message

easily accessible as key-value pairs via Schema.metadata
✅
✅

Small packing size
✅
✅

code complexity
⚠ complexity function overload
✅ similar to the current

performance
✅ slightly more complex defaultArgs
✅ slightly more complex defaultArgs

We should refer to existing popular standards such as annotations for ...

I would not add properties like examples or deprecated. What use do you see for them? For example, JS Doc can be used as a comment to add such informations.

I don't think SQL properties should be included in the valibot package itself.

Weren't the SQL properties the main reason for this feature? Are the properties not standardized? I thought that a Valibot schema could then be sufficient to generate SQL commands.
In general: Where and how do you plan to use this metadata feature?

✅ slightly more complex defaultArgs (at the ErrorMessage position)

How would you implement defaultArgs in this case?

Weren't the SQL properties the main reason for this feature? Are the properties not standardized?

Yes, in fact, there is no standard.
In addition, valibot schema does not contain sufficient information to generate table-building statements SQL. Let's look at a few examples:

In valibot, there is a number() schema,

In MySQL, there are INT, SMALLINT, BIGINT, TINYINT, FLOAT, DOUBLE, DECIMAL types.

In PostgreSQL, there are INT, SMALLINT, BIGINT, DECIMAL, DOUBLE PRECISION, NUMERIC, MONEY types.

When we face the same SQL statement to build a table, there are many differences in the API design of different orm:
CREATE TABLE "Post" (
  "id" SERIAL,
  "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP,
  "updatedAt" TIMESTAMP(3) NOT NULL,
  "title" VARCHAR(255) NOT NULL,
  "content" TEXT,
  "published" BOOLEAN NOT NULL DEFAULT false,
  "authorId" INTEGER NOT NULL,
  PRIMARY KEY ("id")
);

TypeORM Declaration Code
import {
  Entity,
  PrimaryGeneratedColumn,
  Column,
  CreateDateColumn,
  UpdateDateColumn,
} from 'typeorm';

@Entity('Post')
export class Post {
  @PrimaryGeneratedColumn()
  id: number;

  @CreateDateColumn({ type: 'timestamp', default: () => 'CURRENT_TIMESTAMP(3)' })
  createdAt: Date;

  @UpdateDateColumn({ type: 'timestamp' })
  updatedAt: Date;

  @Column({ type: 'varchar', length: 255 })
  title: string;

  @Column({ type: 'text', nullable: true })
  content: string;

  @Column({ type: 'boolean', default: false })
  published: boolean;

  @Column({ type: 'int' })
  authorId: number;
}

MikroORM Declaration Code
import {
  Entity,
  PrimaryKey,
  Property,
  SerializedPrimaryKey,
} from '@mikro-orm/core';

@Entity()
export class Post {
  @PrimaryKey()
  id: number;

  @Property({ type: Date, onCreate: () => new Date() })
  createdAt: Date;

  @Property({ type: Date, onUpdate: () => new Date() })
  updatedAt: Date;

  @Property()
  title: string;

  @Property({ nullable: true })
  content: string;

  @Property({ default: false })
  published: boolean;

  @Property()
  authorId: number;
}

In general: Where and how do you plan to use this metadata feature?

I want to use it as the alternative of reflect-metadata
There are lots of problems when declaring schema in TypeScript's class with reflect-metadata:

Inheritance only, difficult to combine
Need to repeat the type declaration and loses type safety

Specifically, I have two use cases:

declare MikroORM's Entity, I've basically implemented a library valibot-mikro

declare GraphQL schema

The whole community has been trying to move away from reflect-metadata for a few years now.
In the SQL scenario, emerging ORMs are starting to use their own schema syntax:

https://orm.drizzle.team/docs/sql-schema-declaration
https://www.prisma.io/docs/orm/prisma-schema/data-model/models

This is especially true in the world of GraphQL: pothos, nexus, gqtx, grats.
As you can see, there are so many schema builders in the community. The problem is that the current schema builder ecosystem is separate and not as universal as class and reflect-metadata.
I hope that valibot will become the universal schema builder in the TypeScript world.
Once I declared mikro's entity with valibot, no extra work was needed to get json-schema, openapi graphql's ObjectType, Parameters for command line applications.

How would you implement defaultArgs in this case?

/**
 * Returns message and pipe from dynamic arguments.
 *
 * @param arg1 First argument.
 * @param arg2 Second argument.
 *
 * @returns The default arguments.
 */
export function defaultArgs<TPipe extends Pipe<any> | PipeAsync<any>>(
  arg1: ErrorMessage | SchemaMetadata | TPipe | undefined,
  arg2: TPipe | undefined
): [ErrorMessage | undefined, TPipe | undefined, SchemaMetadata | undefined] {
  if (Array.isArray(arg1)) return [undefined, arg1, undefined];

  if (typeof arg1 === 'string' || typeof arg1 === 'function')
    return [arg1, arg2, undefined];

  return [arg1?.message, arg2, arg1];
}

Compare this to the implementation when I use metadata as the last parameter:
/**
 * Returns message and pipe from dynamic arguments.
 *
 * @param args The arguments.
 *
 * @returns The default arguments.
 */
export function defaultArgs<TPipe extends Pipe<any> | PipeAsync<any>>(
  ...args: (SchemaMetadata | TPipe | ErrorMessage | undefined)[]
): [ErrorMessage | undefined, TPipe | undefined, SchemaMetadata | undefined] {
  let message: ErrorMessage | undefined;
  let pipe: TPipe | undefined;
  let metadata: SchemaMetadata | undefined;

  for (const arg of args) {
    if (typeof arg === 'string' || typeof arg === 'function') {
      message = arg;
    } else if (Array.isArray(arg)) {
      pipe = arg;
    } else if (typeof arg === 'object') {
      metadata = arg;
    }
  }

  return [message, pipe, metadata];
}

Thank you very much! I see a problem. The object and tuple schema functions accept a schema as the first optional argument. Since a schema is represented as an object, we can distinguish it from the metadata object.

Hi, I may change the implementation of object and tuple. I plan to remove the rest argument and provide strictObject and objectWithRest besides object. Same for tuple. strictObject and objectWithRest simply reuse object internally. This change has two benefits. On the one hand, the implementation is more modular, which reduces the bundle size when using only object without the rest argument. On the other hand, it reduces the complexity of the implementation and the type overload definition.
If we also remove the pipe argument and introduce the pipe function, we completely eliminate function overloads for schema functions. If we do this, we can consider either passing the metadata in the form of an object as an argument to schema functions (which would be the easiest), or providing metadata actions for the pipe function. However, this would require the metadata actions to be executed first, which degrades the performance of the initialization and increases the bundle size.
I would therefore prefer option 1. Nevertheless, we should also take a look at the DX.

I could imagine that the first optional argument is always the message and the second optional argument is metadata.
// With message and metadata
const Schema = pipe(
  string('Text ...', { description: 'Text ...' }),
  minLength(1)
);

// With just metadata
const Schema = pipe(
  string({ description: 'Text ...' }),
  minLength(1)
);

I prefer to use metadata as argument, it makes valibot more clean when constructing the schema:
// With the new `pipe` function
const UserSchema = pipe(
  object({
    id: pipe(string(), primaryKey(), columnName(user_id)),
    name: pipe(string(), unique()),
    bio: pipe(string(), description('Text ...')),
  }),
  table('users')
)

// With metadata argument
const UserSchema = object(
  {
    id: string({ primaryKey: true, columnName: 'user_id' }),
    name: string({ unique: true }),
    bio: string({ description: 'Text ...' }),
  },
  { tableName: 'users' }
)

Another important point is that when using valibot as a pure schema builder, the ORM itself contains solid validation, and it is not very useful to validate data on top of the ORM.
That said, when I use the medatada feature, I almost no longer need the pipeline to validate the data. Separating the pipeline from medatada seems perfect to me. This actually provides better DX, I don't have to declare pipe over and over again.
The only extra work for the developer is to declare the extra metadata fields while installing valibot, as yup and trpc do:
// env.d.ts
import { MikroSchemaMetadata } from 'valibot-mikro'
import 'valibot'

declare module 'valibot' {
  export interface SchemaMetadata extends MikroSchemaMetadata {}
}

Thank you very much for your feedback! I will get back to you as soon as I have made the proposed changes. Then we can discuss the details of the metadata feature implementation together.

I think I found a better way to define not only metadata.
of function for modifing schemas
const Cat = of(
  object({
    id: of(string(), primaryKey(), columnName('user_id')),
    name: of(string(), unique()),
    loveFish: of(boolean(), description('Does the cat love fish?')),
  }),
  table('cats')
);

This is very similar to pipe, but of works on schema.
Benefits
It's modular;
It's allow to modify oringin schema's type:
Actions for of function can add extra properties to schema.

const loveFish: of(string(), description('Does the cat love fish?'))

loveFish.description // TypeScript will infer that it's string

When work with other library which need schema, this feature will significantly reduce the number of variables and lines of code.
For example, work with MikroOrm and GraphQL :

Without of function:

export const Cat = object(
  {
    id: string({ primaryKey: true, columnName: 'user_id' }),
    name: string({ unique: true }),
    loveFish: boolean({ description: 'Does the cat love fish?' }),
  },
  { tableName: 'cats' }
);

export const CatEntity = toEntity(Cat);
export const CatGraphQLType = toGraphQLType(Cat);

// use entity
const Tom = em.create(CatEntity, { name: 'Tom' });

// use GraphQL type
const schema = new GraphQLSchema({ type: [CatGraphQLType] });

With of function:

const Cat = of(
  object({
    id: of(string(), primaryKey(), columnName('user_id')),
    name: of(string(), unique()),
    loveFish: of(boolean(), description('Does the cat love fish?')),
  }),
  name('Cat')
  asEntity('cats') // implement MikroOrm Entity on `entity` property
  asGraphQLType() // implement GraphQL type on CatSchema self
);

// use entity
const Tom = em.create(Cat.entity, { name: 'Tom' });

// use GraphQL type
const schema = new GraphQLSchema({ type: [Cat] });

As above, we only need one schema object to adapt all libraries, which will greatly improve the development experience.
The of doesn't really have a clear meaning, and we can also use a term like derive, as Rust does.

I've found that having TypeScript accurately infer the metadata type is very helpful in checking the correctness of the program.
const Cat = pipe(
  object({
    id: pipe(string(), primaryKey(), columnName('user_id')),
    name: pipe(string(), unique()),
    loveFish: pipe(boolean(), description('Does the cat love fish?')),
  }),
  name('Cat')
);

expectTypeOf<typeof Cat['name']>().toEqualTypeOf<string>()
const CatEntity = toMikroEntity(Cat); // It should pass

const Dog = object({
  id: string(),
  name: string(),
  loveFish: boolean(),
})

const DogEntity = toMikroEntity(Dog); // It should fail with a type error because Dog does not have a name

This does not seem to be achievable using metadata arguments. So now I think pipe is a better design.
Another idea is to add the with method to the schema:
export interface BaseSchema {
  // ...
   with<T>(modifier: (schema: this) => T): T;
  // ...
}

function _with<T>(this: BaseSchema, modifier: (schema: this) => T): T {
  return modifier(this);
}

const Cat = object({
  id: string().with(primaryKey()).with(columnName('user_id')),
  name: string().with(unique()),
  loveFish: boolean().with(description('Does the cat love fish?')),
}).with(name('Cat'));

This is more readable than pipe, but adds a little bit of bundle size.

I don't understand what toMikroEntity is doing.

This does not seem to be achievable using metadata arguments.

Can you explain this in more detail?

I have a detailed implementation of toMikroEntity at valibot-mikro
When EntitySchema is missing a name, toEntitySchema throws an error. If TypeScript could hint at this missing name error, then we could notice the error much earlier, rather than reporting it at runtime.
In order for TypeScript to do this hint, we need to carry more detailed types on the valibot schema, For example, carrying a name:
function name(
  value: string
): <T extends object>(x: T) => T & { name: string } {
  return (x) => {
    x.name = value
    return x
  }
}

// with pipe
const Cat = pipe(
  object({
    id: pipe(string(), primaryKey(), columnName('user_id')),
    name: pipe(string(), unique()),
    loveFish: pipe(boolean(), description('Does the cat love fish?')),
  }),
  name('Cat')
);

expectTypeOf<typeof Cat['name']>().toEqualTypeOf<string>()

Using pipe, we can modify the type of schema, in this example, we've added the extra name field to Cat.
And I think it's harder to achieve this with metadata argument.
// with metadata argument
const Cat = object(
  {
    id: string(),
    name: string(),
    loveFish: boolean({ description: 'Does the cat love fish?' }),
  },
  { name: Cat }
)

// Is this possible?
expectTypeOf<(typeof Cat)['name']>().toEqualTypeOf<string>()
expectTypeOf<
  (typeof Cat)['entries']['loveFish']['description']
>().toEqualTypeOf<string>()

Thank you for the details! I am quite busy with my studies at the moment. I will probably get back to you next week.

This is still on my list. I will get back to you as soon as I have the time.

There is another reason why I think the pipe scheme is more appropriate:
integer, cuid2, uuid are declared in pipe, and in TypeScript/JavaScript they are validations, but in GraphQL, PostgreSQL they also represent types.
When I use valibot to just declare the schema without performing validation, I still can't avoid using pipe.
Let's try to define a simple table:
// With pipe
const CatSchema = pipe(
  object({
    id: pipe(string(), uuid(), primaryKey()),
    name: pipe(string(), index()),
    age: pipe(number(), integer()),
  }),
  table('Cat')
)

// With metadata argument
const Schema = object(
  {
    id: pipe(string({ primaryKey: true }), uuid()),
    name: string({ index: true }),
    age: pipe(number(), integer()),
  },
  { table: 'Cat' }
)

The pipe solution is neater and easier to read than the metadata argument.

Thank you for that example. I will take it into account when I review everything.

As mentioned in this blog post, I am currently investigating the implementation of a function and promise schema. Feel free to join the discussion in #243. After that, I will focus on the metadata feature.

I expect to be working on this feature next week.

Currently I see 3 options to implement the metadata feature.
Option 1: metadata method
The metadata method would behave like any other method. It takes a schema as its first argument and returns a copy of it with a metadata object property (the second argument) that can take any values. Using generics, this can be made completely typesafe when accessing schema.metadata.
Implementation

Click to show code
import type { BaseIssue, BaseSchema, BaseSchemaAsync } from 'valibot';

/**
 * Schema with metadata type.
 */
export type SchemaWithMetadata<
  TSchema extends
    | BaseSchema<unknown, unknown, BaseIssue<unknown>>
    | BaseSchemaAsync<unknown, unknown, BaseIssue<unknown>>,
  TMetadata extends Record<string, unknown>,
> = TSchema & {
  /**
   * The metadata.
   */
  readonly metadata: TMetadata;
};

/**
 * Adds metadata to a schema.
 *
 * @param schema The schema to add metadata to.
 * @param metadata The metadata to add.
 *
 * @returns The schema with metadata.
 */
export function metadata<
  const TSchema extends
    | BaseSchema<unknown, unknown, BaseIssue<unknown>>
    | BaseSchemaAsync<unknown, unknown, BaseIssue<unknown>>,
  const TMetadata extends Record<string, unknown>,
>(schema: TSchema, metadata: TMetadata): TSchema {
  return { ...schema, metadata };
}

Usage
import * as v from 'valibot';

// Create schema
const UserSchema = v.metadata(
  v.object({
    id: v.metadata(v.pipe(v.string(), v.uuid()), { index: true }),
    name: v.pipe(v.string(), v.nonEmpty(), v.maxLength(32)),
    age: v.pipe(v.number(), v.integer(), v.minValue(0), v.maxValue(100)),
  }),
  { table: 'users' }
);

// Access metadata
const userMetadata = UserSchema.metadata; // { table: 'users' }
const idMetadata = UserSchema.entries.id.metadata; // { index: true }

Advantages

Extreme simple to implement and use
Very powerful as any data is accepted
Small bundle size and fully modular (tree shakable)
No changes in current code necessary
Fully type-safe and easily accessible

Disadvantages

Can lead to function nesting when using multiple methods
Code can quickly look a bit confusing for devs not familiar with Valibot's API

Option 2: metadata argument
Currently, almost any schema accepts a message as the first optional argument. We could extend the implementation to allow metadata as the second optional argument.
Implementation

Click to show code
Example implementation for string schema:
import {
  _addIssue,
  type BaseIssue,
  type BaseSchema,
  type Dataset,
  type ErrorMessage,
} from 'valibot';

/**
 * String issue type.
 */
export interface StringIssue extends BaseIssue<unknown> {
  /**
   * The issue kind.
   */
  readonly kind: 'schema';
  /**
   * The issue type.
   */
  readonly type: 'string';
  /**
   * The expected property.
   */
  readonly expected: 'string';
}

/**
 * String schema type.
 */
export interface StringSchema<
  TMessage extends ErrorMessage<StringIssue> | undefined,
  TMetadata extends Record<string, unknown> | undefined,
> extends BaseSchema<string, string, StringIssue> {
  /**
   * The schema type.
   */
  readonly type: 'string';
  /**
   * The schema reference.
   */
  readonly reference: typeof string;
  /**
   * The expected property.
   */
  readonly expects: 'string';
  /**
   * The error message.
   */
  readonly message: TMessage;
  /**
   * The schema metadata.
   */
  readonly metadata: TMetadata;
}

/**
 * Creates a string schema.
 *
 * @returns A string schema.
 */
export function string(): StringSchema<undefined, undefined>;

/**
 * Creates a string schema.
 *
 * @param message The error message.
 *
 * @returns A string schema.
 */
export function string<
  const TMessage extends ErrorMessage<StringIssue> | undefined,
>(message: TMessage): StringSchema<TMessage, undefined>;

/**
 * Creates a string schema.
 *
 * @param metadata The schema metadata.
 *
 * @returns A string schema.
 */
export function string<
  const TMetadata extends Record<string, unknown> | undefined,
>(metadata: TMetadata): StringSchema<undefined, TMetadata>;

/**
 * Creates a string schema.
 *
 * @param message The error message.
 * @param metadata The schema metadata.
 *
 * @returns A string schema.
 */
export function string<
  const TMessage extends ErrorMessage<StringIssue> | undefined,
  const TMetadata extends Record<string, unknown> | undefined,
>(message: TMessage, metadata: TMetadata): StringSchema<TMessage, TMetadata>;

export function string(
  arg1?: ErrorMessage<StringIssue> | Record<string, unknown>,
  arg2?: Record<string, unknown>
): StringSchema<
  ErrorMessage<StringIssue> | undefined,
  Record<string, unknown> | undefined
> {
  const [message, metadata] =
    typeof arg1 === 'object' ? [undefined, arg1] : [arg1, arg2];
  return {
    kind: 'schema',
    type: 'string',
    reference: string,
    expects: 'string',
    async: false,
    message,
    metadata,
    _run(dataset, config) {
      if (typeof dataset.value === 'string') {
        dataset.typed = true;
      } else {
        _addIssue(this, 'type', dataset, config);
      }
      return dataset as Dataset<string, StringIssue>;
    },
  };
}

Usage
import * as v from 'valibot';

// Create schema
const UserSchema = v.object(
  {
    id: v.pipe(v.string('My custom error message', { index: true }), v.uuid()),
    name: v.pipe(v.string(), v.nonEmpty(), v.maxLength(32)),
    age: v.pipe(v.number(), v.integer(), v.minValue(0), v.maxValue(100)),
  },
  { table: 'users' }
);

// Access metadata
const userMetadata = UserSchema.metadata; // { table: 'users' }
const idMetadata = UserSchema.entries.id.metadata; // { index: true }

Advantages

Extreme simple to use with minimal boilerplate
Very powerful as any data is accepted
Fully type-safe and easily accessible
The additional bundle size is minimal

Disadvantages

Adds additional code to the initial bundle size of each schema (no tree shaking)
Cumbersome to implement and maintain, as each schema must be extended
Makes the schema types more complicated by adding an additional generic
Similar code must be repeated in each schema function

Option 3: Metadata actions

I will add the details later. Please come back in about 2 or 3 hours.

As a fan of MikroOrm and using it in several projects, I would love to be able to define schemas using Valibot, since I currently have to define schemas in MikroOrm and in turn in Valibot.
I believe that the "metadata" should be an "action" that is added to the "pipe". It could be something like the "custom" action but allowing to add "metadata".

NGL I do enjoy the first approach, it feels like a pipe but exlusively for metadatas.
I'm not fully convinced by the second approach, since it could theoretically limit future implementations that we currently don't know yet. The third approach, while being the simplest to use, it feels too limiting and easily become even more complex to maintian (for the devs that will use it) once they need to integrate non-common properties and potentially ditching valibot's metadata entirely and switching back to a INSERT_YOUR_NAME_HERE -> valibot approach (like the mikroorm described).

Been following along as we've been awaiting metadata support in valibot.
Personally a fan of option 1. It matches my mental model for other valibot methods.
Option 2 feels a bit less intuitive, since you are passing a raw object in as an argument, there is no hints in the code at what that argument actually does.
I also like Option 3 --> though restricting to only common metadata properties seems like a non-starter, unless you added a more generic action... something like:
import * as v from 'valibot';

// Create schema
const UserSchema = v.pipe(
  v.object({
    id: v.pipe(v.string(), v.uuid(), v.metadata({ index: true })),
    name: v.pipe(v.string(), v.nonEmpty(), v.maxLength(32)),
    age: v.pipe(v.number(), v.integer(), v.minValue(0), v.maxValue(100)),
  }),
  v.metadata({ table: 'users' })
);

Hope that helps! Thanks for the great work on this lib.

I don't favor using key-value pairs to store metadata:
In my backend app, I use valibot to define both GraphQL Objects and Mikro Entities, each with their own rich metadata.
There are metadata fields with the same name but different types:
import * as v from 'valibot'
import { asField } from '@valibot/graphql'
import { asProperty } from '@valibot/mikro'
import { GraphJSONType } from 'graphql-scalar'
import { JsonType } from '@mikro-orm/core'

export const User = v.object({
  data: v.pipe(
    v.record(v.string(), v.any()),
    asField({ type: GraphJSONType }),
    asProperty({ type: JsonType })
  ),
})

If we use key-value pairs, library users need to declare additional interfaces to get type hints:
// env.d.ts
import { Metadata } from 'valibot'
import { PropertyOptions, EntityMetadata } from '@mikro-orm/core'
import { GraphQLFieldConfig, GraphQLObjectConfig } from 'graphql'
declare module 'valibot' {
  export interface Metadata
    extends PropertyOptions,
      GraphQLFieldConfig,
      GraphQLObjectConfig,
      EntityMetadata {}
}

If we use Metadata actions, each action will come with its own type declaration, and we avoid using the same name for different fields.

I also like Option 3 --> though restricting to only common metadata properties seems like a non-starter, unless you added a more generic action... something like:
import * as v from 'valibot';

// Create schema
const UserSchema = v.pipe(
  v.object({
    id: v.pipe(v.string(), v.uuid(), v.metadata({ index: true })),
    name: v.pipe(v.string(), v.nonEmpty(), v.maxLength(32)),
    age: v.pipe(v.number(), v.integer(), v.minValue(0), v.maxValue(100)),
  }),
  v.metadata({ table: 'users' })
);

Also agree for the case of option 3. But still don't fully understand the drawbacks of this implementation

Less powerful as we can only support common metadata properties by default
Other properties needs to be provided by third party libraries as needed

I don't think that's the downside, I think that's the goal.
As Class Reflect Metadata, does. Each library defines and consumes its own metadata without interfering with the other. An example would be TypeGraphQL + MikroORM
In addition, universally used metadata like descriptions should be provided by the valibot library. Then we don't have to repeat the same thing in different sufferings

Thanks for all your feedback! @xcfox if we choose the pipe option, can we implement it so that we just ignore the metadata actions in pipe and do nothing else? That way the impact on bundle size would be minimal.

if we choose the pipe option, can we implement it so that we just ignore the metadata actions in pipe and do nothing else? That way the impact on bundle size would be minimal.

It's fine. We also need to provide a getMetadata() function which help third party libraries to collect Metadata from pipe.

It's fine. We also need to provide a getMetadata() function which help third party libraries to collect Metadata from pipe.

This would only work if we follow the extraProperties pattern you implemented in #655, but since that does not fit well with the rest of our code, I am not sure I want to do that.
If external libraries take care of implementing specific metadata actions, they could also take care of capturing them with their own code. What do you think?

If external libraries take care of implementing specific metadata actions, they could also take care of capturing them with their own code. What do you think?
Maybe you're right. Actually collecting metadata from the pipeline takes less than a few lines of code, but it does require the library author to learn about valibot

I have created a draft PR for option 3 (metadata actions). Feel free to review it.

I plan to merge #747 today, but I still welcome general feedback.

authors: Sandros94 devcaeg fabian-hiller shayneo xcfox

repo: fabian-hiller/valibot


text:
feature: add formData schema

Hi,
I don't know how this would affect bundle size, but I think it would be interesting to have an API like :
const reportSchema = formData({
  title: string(),
  content: string(),
  photo: file(),
});

It could be useful in server actions (Next.js) and client-side validations in <form>.

I think the idea is cool, but I don't know yet how much sense it makes, because as far as I know FormData can't be typed. Probably the better approach would be to use object and convert FormData to an object before validation.

Here is a code example:
import { email, minLength, object, parse, string } from "valibot"; // 0.75 kB
import { loginUser } from "~/api";

const LoginSchema = object({
  email: string([
    minLength(1, "Please enter your email."),
    email("The email address is badly formatted."),
  ]),
  password: string([
    minLength(1, "Please enter your password."),
    minLength(8, "You password must have 8 characters or more."),
  ]),
});

export default function LoginRoute() {
  async function login(formData: FormData) {
    "use server";
    try {
      const { email, password } = parse(
        LoginSchema,
        Object.fromEntries(formData.entries())
      );
      await loginUser({ email, password });
    } catch (error) {
      // Handle errors
    }
  }

  return (
    <form action={login}>
      <input name="email" type="email" required />
      <input name="password" type="password" required minLength={8} />
      <button type="submit">Login</button>
    </form>
  );
}

Here is a code example:
import { email, minLength, object, parse, string } from "valibot"; // 0.75 kB
import { loginUser } from "~/api";

const LoginSchema = object({
  email: string([
    minLength(1, "Please enter your email."),
    email("The email address is badly formatted."),
  ]),
  password: string([
    minLength(1, "Please enter your password."),
    minLength(8, "You password must have 8 characters or more."),
  ]),
});

export default function LoginRoute() {
  async function login(formData: FormData) {
    "use server";
    try {
      const { email, password } = parse(
        LoginSchema,
        Object.fromEntries(formData.entries())
      );
      await loginUser({ email, password });
    } catch (error) {
      // Handle errors
    }
  }

  return (
    <form action={login}>
      <input name="email" type="email" required />
      <input name="password" type="password" required minLength={8} />
      <button type="submit">Login</button>
    </form>
  );
}

Yes, in this case it's sufficient, but when we need to upload files, we have to encode the file in base64 on the client side, then decode the base64 on the server side to check the size and buffer, so it's quite redundant. A great library exists in zod for these cases, but its implementation seems heavy.

It is not a good idea to convert a file to base64. Instead the native functionality of the <form /> element should be used. I think that it is currently better for Valibot to focus on validation and leave this functionality to other libraries for now.
In fact, I have already implemented such a function for Modular Forms that can handle even complex objects and arrays: https://github.com/fabian-hiller/modular-forms/blob/main/packages/qwik/src/utils/getFormDataValues.ts

It is not a good idea to convert a file to base64. Instead the native functionality of the <form /> element should be used. I think that it is currently better for Valibot to focus on validation and leave this functionality to other libraries for now.
In fact, I have already implemented such a function for Modular Forms that can handle even complex objects and arrays: https://github.com/fabian-hiller/modular-forms/blob/main/packages/qwik/src/utils/getFormDataValues.ts

Yeah I know that it's not a good idea to convert file to base64 but tRPC did not accept FormData :/
Thanks for your reply, I'm closing this ticket.

Yeah I know that it's not a good idea to convert file to base64 but tRPC did not accept FormData :/

I was not aware of that. If there is no other way, I understand the approach. Because of the modularity of Valibot, this functionality can be added quite easily. If someone starts a project for this that builds on Valibot, I'll be happy to link it on our ecosystem page.

source: gharchive/issue

authors: Yovach fabian-hiller

license: MIT

license_type: permissive

license_source: github-api


text:
Modal Component

Feature Description:
A Modal to display Information, that can be closed
Reasons for Feature:
In the current implementation of the Process Mining Tool, a Popup is rendered, when a node is clicked. As this feature is supported in the implementation, it should be added again.
Implementation Details:
Streamlit has no component to display a popup window. In future versions, it will be added. Right now a st.info is displayed, but this is only temporary.
Approach 1:
Try to create a custom modal component using the Streamlit Components API. This approach uses React and other UI Frameworks (Optional). One Problem may be, that custom components are loaded into its own iframe, so building a modal, that is displayed over existing streamlit UI components may not be possible.
Approach 2:
There do exist custom components created by users. Right now, there exists a streamlit_modal component, that can be used.
Approach 3:
Do no use modals to display node information.
Approach 4:
use JavaScript injection in streamlit to create a modallike component.
Related Issues:
References
Streamlit Component API: https://docs.streamlit.io/library/components/components-api
Streamlit Component Template: https://github.com/streamlit/component-template/tree/master/template/my_component
Bootstrap Modal: https://getbootstrap.com/docs/5.3/components/modal/
MaterialUI Dialog: https://mui.com/material-ui/react-dialog/
streamlit_modal Component: https://github.com/teamtv/streamlit_modal
JS injection in Streamlit: https://discuss.streamlit.io/t/injecting-js/22651

Will not be used. Use a st.expander (or other components) to display node information below the buttons

see #23


text:
fix: Remove THREE_SCALE_CONNECT_API and make API routes configurable

Configurable API routes:
dependencyAnalytics.host
dependencyAnalytics.apiKey
dependencyAnalytics.stackReportUIHost

[test]

Just started reviewing :)

source: gharchive/pull-request

authors: arajkumar dvandra mathur07


text:
prod-preview Quick add is'nt display

verfied

license_source: bigquery


text:
Quickstart - Import code - different users are seeing different import options

See the atatched screenshots - it looks the user is asked to specify an organization, only if the user is already in one or more github organizations:

However - our test account does belong to one organization, and that user is not asked to specify an organization:

The same problem is being seen tody - even though the "almightytest" user us now a member of 2 organizations.
What is the threshold for a user account to trigger/see the organization option when importing code for a new project? Thx!

Here are my organizations - what openshift.io sees and what is on github are not in sync

And the repository name pattern drop-down list is not accurate for the organization I picked on the first step.  The burrsutter organization does not have these repositories

The root cause is linked to https://github.com/fabric8io/fabric8-generator/issues/17

authors: burrsutter corinnekrych ldimaggi


text:
Minimal Maven version

Quickstarts currently enforce Maven 3.1.1 by the enforcer plugin and 3.2.1 by <prerequisites> in the main pom.xml.
Which one is correct ?

3.2.1 :)

authors: rajdavies rhuss


text:
collate configuration items

If we want to use some configuration items, we should declare them on the OpenShiftElasticSearchPlugin.
I did some proof-reading.

[merge]

authors: Gallardot jcantrill


text:
Move Query & Format to Top Level of an Action

Action has moved to a struct with members Query, Format & Interaction. This fulfills a few needs:

There is far less duplication of performing queries on an Action since all actions have a query.
The output format of all actions uses the Format
Future Interaction cases that don't rely on any associated types are simpler to implement
Interaction cases map closer to FBSimulatorInteraction.
Help should be easier to define.

lgtm

authors: lawrencelomax marekcirkos

repo: facebook/FBSimulatorControl

license: mit


text:
[TS] Cannot re-export a type when the --isolatedModules flag is provided

Is this a bug report?
No
Description
Previously I used the following file structure for components:
FooComponent/
|- FooComponent.module.scss
|- FooComponent.tsx
|- index.tsx

In my FooComponent.tsx I'd basically define the following:
export interface IFooComponentProps { /* ... */ }
export function FooComponent(props: IFooComponentProps) { /* ... */ }

Furthermore I'd re-export the types in index.tsx as follows:
export { FooComponent, IFooComponentProps } from "./FooComponent";

Unfortunately CRA does not allow me to disable isolatedModules in the tsconfig.json and the compiler is complaining about re-exporting IFooComponentProps. In my use case I have an IFooComponentProps[] in another file that I am using to dynamically create FooComponents. It seems I have to move my props interface into separate files (especially when I use stateful components) which is just unnecessarily increasing the amount of files.
I'd like to know why CRA is enforcing isolatedModules (even automatically writing it to the config if I remove it manually) and how I may fix my design.

Babel is used to compile typescript, but requires isolatedModules to be true. Typescript won't let you re-export types when isolatedModules is true because of this. There's your explanation at least.

I came across a workaround that is really weird to me. Writing export * from "./FooComponent"; allows me to re-export and use the classes and interfaces once again. It seems arbitrary that this works but it serves its purposes in my case.

I'll add that you can kinda do the same thing by manually exporting an alias to the type. Kinda annoying, and maybe there's a better way, but it's what I'm doing when I need it.
import { SomeType } from "./types";
// export { SomeType };
export type SomeType = SomeType;

authors: ChristianIvicevic heyimalex

repo: facebook/create-react-app


text:
react-scripts has vulnerability in dependencies

Hi.
I have an issue with dependencies of react-scripts package.
In the latest version there is a dependency from jest@23.6.0.
But this version of jest is deeply dependent from the braces package.
Here is reported vulnerability in braces package in versions earlier then v2.3.1 https://www.npmjs.com/advisories/786.
But jest@23.6.0. refers to [braces] version before 2.3.1.
This issue should be fixed in jest v24.0.0-alpha.13 and higher.
Can you help to resolve this issue?

This is tracked in https://github.com/facebook/create-react-app/issues/6443 and it will be addressed in create-react-app v3

authors: bugzpodder serhiyzablotskiy


text:
fix(react-scripts): wrong glob for excluded typescript files on webpack config

Using the digitalocean Glob tools i tested the wrong glob and the correct glob, so I changed the code with the correct glob
Wrong glob

Correct glob

Hi @adrielcodeco!
Thank you for your pull request and welcome to our community.
Action Required
In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.
Process
In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.
Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.
If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

I did it. The others errors were not my fault though...


text:
fix: invalid package-lock.json

Seems like some merge conflicts in https://github.com/facebook/create-react-app/pull/12288/files created an invalid package-lock.json

@iansu @mrmckeb sorry for the tag, I can't add reviewers and saw you were both on the original pull request


text:
Bump minors

Bumping everything that had new minor/backwards compatible releases.
Utilizing npm-check

Looks like you're running into the same issue i was with a newer version of webpack-dev-server: https://github.com/facebook/create-react-app/pull/7473#issuecomment-519366863
@iansu @mrmckeb we talked about unpinning some packages in next release. Are we alright unpinning in this PR?

@ianschmitz Yeah and I forgot to update the PR description. Let me know about the pinning and i adjust

I think our plan was to try out unpinning everything in a beta. Which will likely be 4.0 at this point.

@iansu i updated the PR holding back webpack-dev-server for now and tests are passing. I can see if i can pinpoint the version in separate PR.

@iansu Done. Going to check why the docs deploy test failed.
There are more packages to upgrade but they contain incompatible changes so let's do that in separate PR

authors: andriijas ianschmitz iansu


text:
chore: enable pt-BR i18n locale on staging

Motivation
See https://github.com/facebook/docusaurus/issues/3526#issuecomment-888413518
cc @rafaelbmateus thanks :)

Thanks @slorber! Nice

the translations are not yet available, is that it?

will take a look tomorrow ;)

authors: rafaelbmateus slorber

repo: facebook/docusaurus


text:
Add clarifying comments to 'getRemovalRange'

I found some of this code confusing and added comments. Will probably
add more comments while looking at this part of the code.
For now pushing this before I lose track of things. Not in any hurry to merge it.

Probably need to rebase this. Will get around to it in a bit.

repo: facebook/draft-js


text:
go test is failing: no such function: JSON_EXTRACT

Operating system: Ubuntu 20.04
Go version: 1.15.1
Directory: entc/integration
command:  go test ./...
This command is giving following error:
json_test.go:187:
                Error Trace:    json_test.go:187
                                                        json_test.go:97
                Error:          Received unexpected error:
                                no such function: JSON_EXTRACT
                Test:           TestSQLite
FAIL

How to fix this error?

Hey @dilipkk-foyernet! You need to build sqlite with json1 tag - see go-sqlite doc.
BTW, JSON predicates are not yet official in entgo.io docs, so if you use the API under dialect/sql, you should expect a breaking change soon :| (it'll be mentioned in the release note).

repo: facebook/ent


text:
Wrong behavior with EventTarget checking.

I noticed strange behavior with checking event handler. Looks like flow(0.25 flow-bin) recognize e.tagtet as HTMLSelectElement only for first statement within if().
chooseFilter(e: Event) {
  if (e.target instanceof HTMLSelectElement) {
    console.log(e.target);
    this.state.set('filter', e.target.value);
  }
}

e.target.value property not found in e.target
chooseFilter(e: Event) {
  if (e.target instanceof HTMLSelectElement) {
        this.state.set('filter', e.target.value);
        console.log(e.target);
  }
}

Everything is fine with Flow!

Maybe it works this way to avoid side effects affection, but it's not very intuitive.

You are correct, this is by design. The error messaging on this could be better.

function updateHandler(event: Event<HTMLInputElement>) {
}

That's how I had to use, I guess

repo: facebook/flow


text:
Internal Flow error from "export type * from"

I'm assuming that syntax isn't actually expected to work, so obviously not a high priority. I was checking to see since I wasn't sure, and it looks like Flow hits an unexpected error, rather than reporting a syntax error.
export type * from "./foo";

The Flow CLI reports
unexpected parsing exception: Failure("Internal Flow Error! Parsed `export type` into something other than a type alias!")

but the website "Try" page doesn't provide any feedback due to an unhandled exception.

Thanks for the report.
This is a double-whammy and it's on me. At minimum this should be a parse error rather than an internal error. I'll fix this up

Is there an issue to add support for https://github.com/leebyron/ecmascript-export-ns-from ?

authors: gajus jeffmo loganfsmyth


text:
[iOS] Incorrect Memory Leak Detection

I see this as a MEMORY_LEAK. Is there a way to silence this? (using infer v0.12.1):

(NSString *)devicePlatformString {
size_t size;
sysctlbyname("hw.machine", NULL, &size, NULL, 0);
char *machine = malloc(size);
sysctlbyname("hw.machine", machine, &size, NULL, 0); // <= error here
NSString *platform = [NSString stringWithUTF8String:machine];
free(machine);
return platform;
}

Message: error: MEMORY_LEAK
memory dynamically allocated to machine by call to malloc() at line 505, column 21  (char *machine = malloc(size);) is not reachable after line 506, column 5 (sysctlbyname("hw.machine", machine, &size, NULL, 0);)
Previous issue was closed #52 , but it contains a new comment.

There isn't a way to silence individual bugs at the moment.

authors: akkrat dulmarod

repo: facebook/infer


text:
mocks don't seem to be registered correctly

🐛 Bug Report
When setting up a mockResolvedValue, a mockImplementation, etc. for an async function (such as data fetching) inside beforeEach or a test the mock works as expected.
However when the mock is set up in beforeAll the defined return value of the mock is not returned and an error is thrown. The same error is thrown when setting up the mock at the top of the module.
Since we're setting up only one test the semantics of beforeEach and beforeAll should be the same.
Error message:

Error: Uncaught [TypeError: Cannot read property 'then' of undefined]

To Reproduce
See test in this repo
Expected behavior
When setting up the mock

at the top of the module, the mock implementation / resolved value should still work
in the beforeAll hook the mock should work the same way as the beforeEach (at least in our context, because we only have one test in this case)

Link to repl or repo (highly encouraged)
https://github.com/lehnerchristian/jest-test
envinfo
npx: installed 1 in 0.795s

  System:
    OS: Linux 5.8 Ubuntu 20.04.2 LTS (Focal Fossa)
    CPU: (16) x64 Intel(R) Xeon(R) W-10885M CPU @ 2.40GHz
  Binaries:
    Node: 16.0.0 - ~/.nvm/versions/node/v16.0.0/bin/node
    Yarn: 1.22.10 - ~/.nvm/versions/node/v16.0.0/bin/yarn
    npm: 6.14.13 - ~/.nvm/versions/node/v16.0.0/bin/npm

Is there anything unclear in this bug report?

Hey, @SimenB and @cpojer ! Not sure if this is really a bug or if we only did something wrong here. Can you shed some light on this one? Happy to close if this is an issue on our side. Here's the link to the repo in which we reproduced the error: https://github.com/lehnerchristian/jest-test/blob/main/src/CharacterName.test.js
Thanks in advance!

https://github.com/facebook/create-react-app/issues/10571#issuecomment-784923197
@lehnerchristian

This works indeed. Thanks for the link. Not sure how useful it is to have any mocks having their fake implementations removed, because why write a fake implementation in the first place then.
So this means that if I try to set a mock in the beforeAll test lifecycle it gets overridden if this option is set before running the first test. This also applies for setting the mocks globally within the test file. Only beforeEach and setting the mocks in the test work, because the run after the resetMocks call?
Or in short:
1. beforeAll
2. resetMocks
3. beforeEach

Is this correct?

Just to add to @lehnerchristian point about the usefulness of that flag: Could someone maybe explain that actually, because I also have a hard time in understanding what the actual use of that flag should be. Basically (as I understand it now) it completely disabled all beforeAll blocks... which is something I absolutely would not expect.

authors: gasserkl idler8 lehnerchristian

repo: facebook/jest


text:
Discussion about async operations, network requests and mocking.

I have some thoughts and ideas about "integration" tests. Would love to start an open discussion and get some insight on mocking in general. Feel free to close if this isn't the place to have this.
I posted a question on StackOverflow a while ago, read something like this.

I'm interested in creating full mocked unit tests, as well as integration tests that check if some async operation has returned correctly. I'd like one command for the unit tests and one for the integration tests that way I can run them separately in my CI tools. What's the best way to do this? Tools like mocha, and jest only seem to focus on one way of doing things.

It's my opinion that "mocking" a network request isn't the answer. It's time consuming and can not be accurate. In a perfect world what I want is the ability to make network requests (or any async request) and watch my tests pass, then store the payloads locally in the project. This would allow any API changes or changes to the actual payload to be automatic. I can write my code to produce the correct response back from the request, then I could run the test from then on mocked with this automatic response that was cached.
I've seen people pointed around to the jest.disableAutomock function as a solution. I don't think that's the solution. I think that a jest --intergation functionality is necessary to truly test async operations without mocking.
I truly don't understand how you can write a database query, mock it get your test working and think that that's the end of it without ever actually testing the request hitting the database.
I think actually making the async requests and automatically caching, generating the mocks for you all the responses is the way to go. Looking for criticism and feedback, am I missing the full picture? Is jest just primarily for front end? Does this already exist?

Since Jest 15 automocking is disabled by default, so nothing gets mocked unless you tell Jest to do so. You can have another folder for integration tests, e.g. look at integration_tests dir inside Jest repo.
Also nothing prevents you to create test and test-integration npm scripts, running different configs with --config flag.

@kentaromiura talked about this before, it's similar to snapshots: find a way to record remote data through Jest. @reggi can you design an API that makes sense either within Jest or as a third-party module?

I put this together really quick to try and get at what I want. Ideally both flags would fill 100% coverage.
Here's the test:
import fetch from 'node-fetch'
import memoeyesAsync from './memoeyes'

async function getGoogle() {
  let res = await fetch('https://google.com')
  let body = await res.text()
  return body.split('google').length
}

function add(a, b) {
  return a + b;
}

getGoogle = memoeyesAsync({getGoogle}, process.env.TEST_TYPE, {filename: './memoeyes-cache.js'})

describe ('x', () => {
  it('should add two numbers', () => {
    expect(add(1, 2)).toBe(3)
  })
  it('should work', async () => {
    let google = await getGoogle()
    expect(google).toBe(79)
  })
})

Here's the function that caches the result.
import Datastore from 'nedb'
import Promise from 'bluebird'

export default function memoeyesAsync (fnObj, testType, nedb) {
  let fnName = Object.keys(fnObj)[0]
  let fn = Object.values(fnObj)[0]
  nedb.autoload = true
  let db = new Datastore(nedb)
  db.update = Promise.promisify(db.update)
  db.find = Promise.promisify(db.find)
  const INTERGRATION = (testType === 'intergration')
  const UNIT = (testType === 'unit')
  if (INTERGRATION) {
    return async function (...args) {
      let value = await fn(...args)
      let results = await db.find({fnName, args})
      if (results.length == 0) await db.insert({fnName, args, value})
      return value
    }
  } else if (UNIT) {
    return async function (...args) {
      let results = await db.find({fnName, args})
      if (results && results[0] && results[0].value) return results[0].value
      throw new Error('need intergration')
    }
  }
  return false
}

So if you run TEST_TYPE=intergration jest it makes the network request and if you run TEST_TYPE=unit jest it uses the cached saved version of the function result so the test passes, however it does skip over the entire function. 😬 🤔 🌈
This is the kind of functionality I'm looking for.

I vaguely remember a talk @indutny gave on llnode that may give the ability to store the results of every function call so I could hypothetically get a copy of all returned function values, and their place in the code, so we don't have to go purely functional.

Hello everyone!
I'm sorry, but it would be hard to do it with llnode. llnode is a plugin for debugger, not a profiling or general monitoring tool.

Would be nice that in UNIT mode, it would do a jest.fn() or another mock. Maybe some way to have the first call do an integration test might make sense?

One thing is preventing me from coming up with another viable API for this.
I have a conceptual issue with is the following:
Let's say I have the following function.
doSomethingAsync() // like network call

But the argument to this function is a class instance.
const instanceOfData = new Data()
doSomethingAsync(instanceOfdata)

I have no way of storing the input arguments doSomethingAsync because I am incapable of stringifying instanceOfdata and playing it back.
Does anyone have an idea on how we can stringify any set of argument, store them to the file-system then call the same function with the stored arguments to produce the same output?

For network requests, something like nock and its recording might work (see also #6081), /cc @palmerj3
Not too sure about "other async work" such as FS or DB. I suppose DB should just work as that's often TCP or something, but intercepting FS access seems overkill.
I think this issue is a bit vague. It's more a discussion which seems to have died out.
If people come across this, follow #6081 for HTTP mocking, and open up a new issue if you want some sort of playback for IO that's not network.
(happy to reopen if people disagree!)

authors: Haroenv SimenB cpojer indutny reggi thymikee


text:
Babel config not loading correctly for --projects.

🐛 Bug Report
Babel config is not loaded correctly for --projects, e.g.
/mono
  /packages
    /pkg1
      .babelrc.js

jest --projects packages/pkg1  <-- fails
cd packages/pkg1 && jest  <-- passes
To Reproduce
The examples/react in the jest monorepo itself actually does exhibit this bug.
The bug is hidden because of babelrcRoots: ['examples/*'] in the higher-level babel.config.js.
Expected behavior
Behavior should be the same as running jest in the project's dir, e.g. cd examples/react && jest
Details to reproduce
In this jest repo itself....

Remove babelrcRoots: ['examples/*'] in top-level babel.config.js.
Run yarn jest --projects examples/react/  <-- fails with message below
Run pushd examples/react && yarn jest <-- passes

Failure message
 FAIL  examples/react/__tests__/CheckboxWithLabel-test.js
  ● Test suite failed to run

    SyntaxError: jest/examples/react/__tests__/CheckboxWithLabel-test.js: Unexpected token, expected "," (11:23)

       9 |   // Render a checkbox with label in the document
      10 |   const checkbox = TestUtils.renderIntoDocument(
    > 11 |     <CheckboxWithLabel labelOn="On" labelOff="Off" />
         |                        ^
      12 |   );
      13 |
      14 |   const checkboxNode = ReactDOM.findDOMNode(checkbox);

      at _class.raise (../../node_modules/@babel/parser/lib/index.js:4051:15)

Analysis
The failure occurs because the babel config is not loaded with the proper cwd...the top-level cwd is used, instead of the project's rootDir.  See here and here.
Also, when jest is loading configs, cwd in the project configs are set to the top-level (original) cwd here...not sure if that's the intention, seems like they should be set to each project's rootDir.

After some more analysis...
Jest runs --projects with cwd at top-level, instead of changing cwd to each project.  That makes running all projects together at top level behave differently than running each individually, at least with regard to babel, because .babelrc.js in individual projects apply when running the project individually, but don't apply when the project is run at top level via --projects.  The behavior is inherited from babel's config loading behavior.
The .babelrc.js files will apply if the project dirs are added to babelrcRoots in the top-level babel.config.js, like how it is done in this repo.
I think it is confusing that the behavior is different, but I haven't convinced myself that it's wrong, so I'll close this out.  Thanks!

I think the question is: what is the intention of --projects?  Should it be exactly the same as running each individually?  (For me, that was my expectation, I simply want to run all my projects' tests in the same jest instance.)  Or should it behave like babel, where the behavior is different because cwd is different?  (For me, that's confusing, but maybe some people expect that.)
Another consideration is a case like below which I would expect to "just work" when running --projects or individually.
/root
  babel.config.js  (or .babelrc.js?)
  globalSetup.js
  /prj-1
    .babelrc.js
  /prj-2

What are the proper babel programmatic options for files outside of project dir?  Maybe {cwd: prj.rootDir , rootMode: 'upwards-optional'} would work for most cases, with overrides possible via https://github.com/facebook/jest/pull/7288?  Babel docs suggest using upward for monorepos, but it makes more sense to me to fall back to babelRoot == prj.rootDir if there isn't a babel.config.js.  (I think this would be msotly backwards-compatible with current behavior, because, if there is a babel.config.js, then it would apply and it would need to contain .babelrc.js locations in babelrcRoots for them to apply.)

It was, AFAIK, designed to run different projects in the same instance - e.g. react and jest at the same time. It was not designed (although I think way more used as) a way to apply different jest config to different tests/files within the same project. So the answer to "Should it be exactly the same as running each individually?" is "yes".

authors: SimenB bradfordlemley


text:
Fixed object matcher

Summary
Updated the object matcher utility to use hasOwnProperty independent of the object passed.
Before, expect(Object.create(null)).toMatchObject(someObject) would throw an error, as Object.create(null) lacks the object prototype.
It appears that you all had already noticed this as you had created a helper for it, but weren't actually using it.
Test plan
yarn run jest --silent && yarn run test-examples --silent

Codecov Report

Merging #3799 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master    #3799   +/-   ##
=======================================
  Coverage   57.58%   57.58%
=======================================
  Files         194      194
  Lines        6776     6776
  Branches        6        6
=======================================
  Hits         3902     3902
  Misses       2871     2871
  Partials        3        3

Impacted Files
Coverage Δ

packages/jest-matchers/src/utils.js
100% <100%> (ø)
:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 03bb9b3...bd41748. Read the comment docs.

authors: codecov-io jzeiders


text:
don't use snapshot for open handles

Summary
Node 10.2 seemingly added an extra handle breaking ci. So no more snapshot, this is easier

Test plan
Green CI

Codecov Report

Merging #6296 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #6296   +/-   ##
=======================================
  Coverage   63.54%   63.54%
=======================================
  Files         221      221
  Lines        8502     8502
  Branches        3        4    +1
=======================================
  Hits         5403     5403
  Misses       3098     3098
  Partials        1        1

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec7e8fb...d64c716. Read the comment docs.

authors: SimenB codecov-io


text:
perf: consolidated fs operations for config finding

Summary
While walking through some of the jest config finding logic - I noticed some redundant fs operations.
This pr is a performance improvement focused on reducing fs operations.
Test plan
No breaking changes intended + tests pass?

@thymikee for sure, this is still WIP, when i have time to wotk through those errors

@hulkish any chance of reviving this? Any perf improvements are always very much welcome! 🙂 Lots of conflicts now since we've migrated to TS, but hopefully they're not too bad to resolve

@hulkish ping 🙂 Are you still interested in working on this?

authors: SimenB hulkish


text:
chore: add caching to azure pipelines

Summary
Hopefully this'll speed up CI.

Test plan
Green CI with some nice cache output?

Ergh

@willsmythe could you provide some info here? 🙏

I checked the telemetry and the problem doesn’t appear to be specific to your pipeline/job. Can you try re-running the pipeline and see what happens? This might have been an intermittent problem that impacted the agent this particular job got assigned. I’ll follow-up with the engineering team …
Will
From: Simen Bekkhus notifications@github.com
Sent: Thursday, August 22, 2019 9:49 AM
To: facebook/jest jest@noreply.github.com
Cc: Will Smythe wismythe@microsoft.com; Mention mention@noreply.github.com
Subject: Re: [facebook/jest] chore: add caching to azure pipelines (#8866)
Ergh
[image]https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fuser-images.githubusercontent.com%2F1404810%2F63519948-2139a100-c4f4-11e9-940a-5500c8b72e5d.png&data=02|01|wismythe%40microsoft.com|13495e9fb66a4eae565108d727078244|72f988bf86f141af91ab2d7cd011db47|1|0|637020785522614914&sdata=P6FskGYyLDkgZaI5TRD%2F4FagFDtLCnncnkzdEeLMJak%3D&reserved=0
@willsmythehttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fwillsmythe&data=02|01|wismythe%40microsoft.com|13495e9fb66a4eae565108d727078244|72f988bf86f141af91ab2d7cd011db47|1|0|637020785522624909&sdata=7bz69jaORfOJld92WWKr9phWsqA8vxELok5rUOsssVs%3D&reserved=0 could you provide some info here? 🙏
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ffacebook%2Fjest%2Fpull%2F8866%3Femail_source%3Dnotifications%26email_token%3DAATDDDDX5D7R7W3QDJ3GGELQF2KNLA5CNFSM4IOVFW2KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD45EYSA%23issuecomment-523914312&data=02|01|wismythe%40microsoft.com|13495e9fb66a4eae565108d727078244|72f988bf86f141af91ab2d7cd011db47|1|0|637020785522624909&sdata=2fCvm53bcP%2B8b75YIGUW5U73NH0NPS0wwOa87ctHeBE%3D&reserved=0, or mute the threadhttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAATDDDG7LFPQ5J22MVZFUG3QF2KNLANCNFSM4IOVFW2A&data=02|01|wismythe%40microsoft.com|13495e9fb66a4eae565108d727078244|72f988bf86f141af91ab2d7cd011db47|1|0|637020785522634903&sdata=Gf%2FUL4hkLfrz5NMTIqHfLYHDrdWcHFhQjllTYbzXyL4%3D&reserved=0.

Cool, thanks! I've clicked the button. I guess a new run will also let me verify if the cache worked for the mac and windows runs, as they completed successfully 🙂

Doesn't seem like the build wants to start for whatever reason. Maybe just a queue?

Cool, thanks! I've clicked the button. I guess a new run will also let me verify if the cache worked for the mac and windows runs, as they completed successfully 🙂

@SimenB - do you recall which button you clicked? Was it in the GitHub Checks tab, or in Azure Pipelines? Asking because I see a new run (https://dev.azure.com/jestjs/jest/_build/results?buildId=3224&view=results), but it looks stalled. I've asked the engineering team here about this as well ...

Looks stuck, yeah. I clicked the Rebuild button here:

The build never triggered... Any idea what's going on?

I'll just rebase this, which should trigger a build.

Codecov Report

Merging #8866 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #8866   +/-   ##
=======================================
  Coverage   64.13%   64.13%
=======================================
  Files         275      275
  Lines       11627    11627
  Branches     2845     2844    -1
=======================================
  Hits         7457     7457
  Misses       3544     3544
  Partials      626      626

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6c5d96a...6b48a4d. Read the comment docs.

Ish 3 min install now on all OSes - hopefully we'll see an improvement

Yeah, it's about a minute faster, which is great:

Caching node_modules istead like @willsmythe mentions will be possible soon:tm: will probably be even faster, as the install step should be even shorter

@SimenB - good to hear. The issue with the failed job (which only happens for some orgs on the very first job using caching) is being investigated. Looks to be a race condition. I haven't heard back from the team on the second issue (rebuild run fails to start).

authors: SimenB codecov-io willsmythe


text:
Fix IntelliJ plugin on IJP 242

This PR fixes the issues in IJ 24.2, where the ktfmt crashes when being loaded and is essentially non-functional. It also removes a deprecated API usage in the same file.
Closes #494

Would you mind rebasing it?

Done

Done and green

repo: facebook/ktfmt


text:
docs: added exportJSON function to the ExtendedTextNode plugin code

Added the exportJSON function to solve the problem of errors when calling the editor.getEditorState().toJSON() method when using official documents.

Thank you!

authors: acywatson ninawangyimei

repo: facebook/lexical


text:
[lexical-table] Bug Fix: get table-cell background selection color from a class

Description

get table-cell background selection color from a class
I couldn't find where tableCellSelected is used, my guess is that it's not so I overwrote the class
and I don't think that tableCellSelectedOverwrite is a good name so I'm open for other suggestion it's there as a placeholder for now.
Closes #6655
Test plan

I left the old logic in, because I'm not about another way to set a default value (new to the code base)
I'll update the failing tests once the logic gets approved

The CSS class is probably a leftover from the experimental table, which we deleted a year/year and a half ago. I think it's fine to reuse that CSS class, I did the same with reintroducing the hover add column/row buttons and reusing the existing CSS classes. I'm not in favour of adding an extra override CSS class, as long as you pick the CSS class name from the config.theme object use the existing CSS Class, also +1 for the test.

I agree that an override class is not necessary. Should be up to the user to make sure their configuration of classes and styles composes correctly.

Would be great to see this fix in the next release!

@hamza221 are you interested in finishing this one?

I'll send a commit tomorrow

🤞

Were you planning to finish this PR?

! I believe there's one mistake in the test changes, and that it can be simplified further by not using element.style at all

I'm okay with someone else taking over, I'm Afk until 2025

I'll take a look at finishing this after #6759

Thank you @etrepum

@ivailop7 might want to take a look at this one


text:
what generations means in model_generators's model ? eg.ImplicitIntentSourceGenerator.json

The model attribute of model_generators in the document only provides sources, sinks, and propagation, and the value of generations is not found.
But in ImplicitIntentSourceGenerator.json, generations are used. I want to know what is the difference between generations and sources, or what generations means, is it to initialize a class as a source, or...
ImplicitIntentSourceGenerator.json ->
{
  "model_generators": [
    {
      "find": "methods",
      "where": [
        {
          "constraint": "parent",
          "inner": {
            "constraint": "name",
            "pattern": "Landroid/content/Intent;"
          }
        },
        {
          "constraint": "name",
          "pattern": "\\<init\\>"
        },
        {
          "constraint": "not",
          "inner": {
            "constraint": "signature",
            "pattern": ".*Ljava/lang/Class;.*"
          }
        }
      ],
      "model": {
        "**generations**": [
          {
            "kind": "IntentCreation",
            "port": "Argument(0)"
          }
        ]
      }
    }
  ]
}

rules
I found that there are many functions in use that are not reflected in the document.
For example model's key = generations,The relationship between multi_sources and partial_sinks in rules.json is And or or, for example, User input flows into implicitly launched intent rule, the text means that external input is passed to new Intent(...) and then launched intent.
I think the process should be
1, User Input -> Intent test = this.getIntent();
2, Intent ha = new Intent(test);
3, startActivity(ha);
But multi_sources seems to be an or relationship,
1, User Input-> Intent test = this.getIntent();
3, startActivity(ha);
or
2, Intent ha = new Intent(test);
3, startActivity(ha);
{
    "name": "User input flows into implicitly launched intent",
    "code": 3,
    "description": "Values from user-controlled source may eventually flow into an implicit intent and intent launcher",
    "multi_sources": {
      "a": [
        "IntentCreation"
      ],
      "b": [
        "FragmentUserInput",
        "ActivityUserInput",
        "ReceiverUserInput",
        "ServiceUserInput"
      ]
    },
    "partial_sinks": [
      "LaunchingComponent"
    ]
  },

Hi @gitWK86, thanks for reaching out.
See https://github.com/facebook/mariana-trench/issues/48#issuecomment-936686049 for the difference between source and generation.
Regarding the rules: those are multi-source sink rules, which are currently not described in the documentation. cc @yuhshin-oss

Thanks arthaud!

repo: facebook/mariana-trench


text:
Move flipComparison function to ExpressionUtils

DomainTranslator and RelationPlanner both use the flipComparison function, so move it to ExpressionUtils.

Will add this commit to #3072

Merged. Thanks!

authors: cberner yuananf

repo: facebook/presto


text:
How to support https request?

React Native version is 0.33.1
RCTBundleURLProvider.m
static NSURL *serverRootWithHost(NSString *host)
{
 return [NSURL URLWithString:
 [NSString stringWithFormat:@"http://%@:%lu/",
 host, (unsigned long)kRCTBundleURLProviderDefaultPort]];
}
I want to use https request, but when i change http to https in the function serverRootWithHost, cant connect to the server.
how to support https request?

I would try asking this on Stack Overflow - these issues are better for reporting bugs in React Native and this is more of a "how do I hack React Native to do X" question.

authors: jsntghf lacker

repo: facebook/react-native


text:
Black Radius Images on Android

Whenever I add borderRadius to an Image style, it becomes full black circle.

React Native version: 0.43.2
Platform: Android
Development Operating System: OSX
Dev tools: Android Studio

Hi, have you solved this issue? I am facing the same one like this. This only happens on android 5.0, and the Image component is nested inside a FlatList.

I have this issue as well.

+1

me too...

authors: beeant emretekince gameboyVito ol2dd tsaohucn


text:
iOS: Multiple animations which use "useNativeDriver" do not work if preceded by a delay.

[x] I have reviewed the documentation
[x] I have searched existing issues
[x] I am using the latest React Native version

On iOS, if attempting to animate a text component and a view component in parallel, preceded by an Animated.delay (or setTimeout), the text component will not animate:
doAnimate = () => {
    Animated.sequence([
      // Removing the delay here will fix the issue.
      Animated.delay(1000),
      Animated.parallel([
        Animated.timing(this.squareAnimation, { toValue: 1, useNativeDriver: true, duration: 3000 }),
        // If you remove "useNativeDriver" below, the animation works.
        Animated.timing(this.textAnimation, { toValue: 1, useNativeDriver: true, duration: 3000 }),
      ])
    ]).start();

    // This also fails to animate the text component:
    // setTimeout(() => {
    //   Animated.parallel([
    //     Animated.timing(this.squareAnimation, { toValue: 1, useNativeDriver: true, duration: 3000 }),
    //     Animated.timing(this.textAnimation, { toValue: 1, useNativeDriver: true, duration: 3000 }),
    //   ]).start();
    // }, 1000);
}

  componentDidMount() {
    setTimeout(this.doAnimation, 3000);
  }

  render() {
    return (
      <View style={styles.container}>
        <Animated.Text style={[styles.textContainer, {
          opacity: this.textAnimation
        }]}>
          Welcome to React Native!
        </Animated.Text>
        <Animated.View style={[styles.squareContainer, {
          opacity: this.squareAnimation
        }]}>
        </Animated.View>
      </View>
    );
  }

If you remove "useNativeDriver" from the "textAnimation", the animation works. Also, making the textAnimation interpolate from viewAnimation does not work (that's how my code originally worked, but I broke them out into separate animations to test what was going wrong).
Environment
Environment:
OS: macOS High Sierra 10.13.3
Node: 8.9.4
Yarn: 1.5.1
npm: 5.6.0
Watchman: Not Found
Xcode: Xcode 9.2 Build version 9C40b
Android Studio: 3.0 AI-171.4443003
Packages: (wanted => installed)
react: ^16.3.0-alpha.1 => 16.3.0-alpha.3
react-native: 0.54.2 => 0.54.2
Steps to Reproduce
See code snippet above, alternatively you can check out my example repo:
git clone https://github.com/dannycochran/animationBug
cd animationBug
yarn install
react-native run-ios

Expected Behavior
The animations for both the View and Text component work, as they do on Android.
Actual Behavior
The text opacity never animates.

Same having problems with Animated and setTimeout, sometimes setTimeout fires early.
Debuging using a Android phone.

This also happens if the delay is set in Animated.timing directly instead of Animated.delay.
RN 0.55.4

Is there any update on this @lnikkila ?

I’m not a maintainer, unfortunately I don’t know when this is going to be fixed. Since it doesn’t result in a major loss of functionality and there’s a workaround it probably doesn’t have a very high priority. I’m sure pull requests are welcome though!

Also noticed this issue, and only for

I have same problem

Same problem expo sdk 38 react rative 0.62

authors: BonnieMilian dannycochran georgeMorales lnikkila uguraktas vitto-moz yezarela


text:
Icon be lost when build on real device

[ ] Review the documentation: https://facebook.github.io/react-native
[ ] Search for existing issues: https://github.com/facebook/react-native/issues
[ ] Use the latest React Native version: https://github.com/facebook/react-native/releases
[ ] Run react-native info in your terminal and paste its contents under "Environment"
[ ] Let us know how to reproduce the issue. Include a code sample, share a project, or
share an app that reproduces the issue using https://snack.expo.io/

Environment

Description

Steps to Reproduce

Expected Behavior

Actual Behavior

I try build on real device (HTC) but icon, image local not display, many people also problem equivalent, Help!, Thank