From 1b6a1f990be46e5e6b541d9cadd0b4df4957be5a Mon Sep 17 00:00:00 2001 From: AdRiley Date: Fri, 25 Oct 2024 11:16:52 +0100 Subject: [PATCH 01/13] Enable SQLServer Sort Feature and associated tests (#11379) * Auto-commit work in progress before clean build on 2024-10-15 12:59:18 * checkpoint * Clean * Cleaner * cleaner * Cleaner * Green * stash * stash * Fix sort * Sub 300 failures * More passing tests * Auto-commit work in progress before clean build on 2024-10-22 09:00:41 * SQLServer green 265 * remove dead code * Add doc * Add generate_column * Refactor * refactor * Refactor * Refactor * Add is_operation_supported_fn * Fix * Fix * Fix * Code review changes * Code review feedback * typos * Add comment --- .../Redshift/Internal/Redshift_Dialect.enso | 4 + .../Database/0.0.0-dev/src/Dialect.enso | 8 ++ .../src/Internal/Base_Generator.enso | 7 +- .../0.0.0-dev/src/Internal/IR/Context.enso | 6 +- .../Internal/Postgres/Postgres_Dialect.enso | 4 + .../src/Internal/SQLite/SQLite_Dialect.enso | 4 + .../src/Internal/SQLServer_Dialect.enso | 84 ++++++++++++++++++- .../src/Internal/SQLServer_Type_Mapping.enso | 4 +- .../src/Internal/Snowflake_Dialect.enso | 4 + test/AWS_Tests/src/Redshift_Spec.enso | 3 +- test/Microsoft_Tests/src/SQLServer_Spec.enso | 3 +- test/Snowflake_Tests/src/Snowflake_Spec.enso | 3 +- .../Add_Row_Number_Spec.enso | 2 +- .../src/Common_Table_Operations/Main.enso | 2 +- .../Common_Table_Operations/Nothing_Spec.enso | 8 +- .../src/Database/Postgres_Spec.enso | 3 +- .../Table_Tests/src/Database/SQLite_Spec.enso | 3 +- .../src/In_Memory/Common_Spec.enso | 4 +- 18 files changed, 134 insertions(+), 22 deletions(-) diff --git a/distribution/lib/Standard/AWS/0.0.0-dev/src/Database/Redshift/Internal/Redshift_Dialect.enso b/distribution/lib/Standard/AWS/0.0.0-dev/src/Database/Redshift/Internal/Redshift_Dialect.enso index 1907fdefda12..0ca6d251e926 100644 --- a/distribution/lib/Standard/AWS/0.0.0-dev/src/Database/Redshift/Internal/Redshift_Dialect.enso +++ b/distribution/lib/Standard/AWS/0.0.0-dev/src/Database/Redshift/Internal/Redshift_Dialect.enso @@ -224,6 +224,10 @@ type Redshift_Dialect _ = value_type False + ## PRIVATE + generate_column_for_select self base_gen expr:(SQL_Expression | Order_Descriptor | Query) name:Text -> SQL_Builder = + base_gen.default_generate_column self expr name + ## PRIVATE ensure_query_has_no_holes : JDBC_Connection -> Text -> Nothing ! Illegal_Argument ensure_query_has_no_holes self jdbc:JDBC_Connection raw_sql:Text = diff --git a/distribution/lib/Standard/Database/0.0.0-dev/src/Dialect.enso b/distribution/lib/Standard/Database/0.0.0-dev/src/Dialect.enso index bc8910f7b46e..86372fb0dbb4 100644 --- a/distribution/lib/Standard/Database/0.0.0-dev/src/Dialect.enso +++ b/distribution/lib/Standard/Database/0.0.0-dev/src/Dialect.enso @@ -269,6 +269,14 @@ type Dialect _ = [base_table, key_columns, resolved_aggregates, problem_builder] Unimplemented.throw "This is an interface only." + ## PRIVATE + Generates a column for the given expression for use in the SELECT clause. + Used for databases where the expression syntax is different in the SELECT clause + to the syntax in the WHERE clause + generate_column_for_select self base_gen expr:(SQL_Expression | Order_Descriptor | Query) name:Text -> SQL_Builder = + _ = [base_gen, expr, name] + Unimplemented.throw "This is an interface only." + ## PRIVATE ensure_query_has_no_holes : JDBC_Connection -> Text -> Nothing ! Illegal_Argument ensure_query_has_no_holes jdbc:JDBC_Connection raw_sql:Text = diff --git a/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/Base_Generator.enso b/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/Base_Generator.enso index 61100cecc755..5f30a6fa867f 100644 --- a/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/Base_Generator.enso +++ b/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/Base_Generator.enso @@ -166,6 +166,10 @@ type SQL_Generator base_expression = self.generate_expression dialect order_descriptor.expression base_expression ++ collation ++ order_suffix ++ nulls_suffix + ## PRIVATE + default_generate_column self dialect expr:(SQL_Expression | Order_Descriptor | Query) name:Text -> SQL_Builder = + self.generate_expression dialect expr ++ alias dialect name + ## PRIVATE Generates SQL code corresponding to a SELECT statement. @@ -175,11 +179,10 @@ type SQL_Generator generate_select_query_sql : Dialect -> Vector (Pair Text SQL_Expression) -> Context -> SQL_Builder generate_select_query_sql self dialect columns ctx = gen_exprs exprs = exprs.map (self.generate_expression dialect) - gen_column pair = (self.generate_expression dialect pair.second) ++ alias dialect pair.first generated_columns = case columns of Nothing -> SQL_Builder.code "*" - _ -> SQL_Builder.join ", " (columns.map gen_column) + _ -> SQL_Builder.join ", " (columns.map (c-> dialect.generate_column_for_select self expr=c.second name=c.first)) from_part = self.generate_from_part dialect ctx.from_spec where_part = (SQL_Builder.join " AND " (gen_exprs ctx.where_filters)) . prefix_if_present " WHERE " diff --git a/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/IR/Context.enso b/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/IR/Context.enso index 64d9796edc02..8ba9ff58b59a 100644 --- a/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/IR/Context.enso +++ b/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/IR/Context.enso @@ -107,14 +107,12 @@ type Context takes precedence, but if any orderings were already present they are also taken into account to break ties within the new ordering. - In practice this means, that the old orderings are preserved, but the new - ones are added to the beginning of the list so that they take precedence. - Arguments: - new_orders: The new ordering clauses to add to the query. add_orders : Vector Order_Descriptor -> Context add_orders self new_orders = - Context.Value self.from_spec self.where_filters new_orders+self.orders self.groups self.limit self.extensions + deduped_orders = new_orders+self.orders . distinct .expression + Context.Value self.from_spec self.where_filters deduped_orders self.groups self.limit self.extensions ## PRIVATE diff --git a/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/Postgres/Postgres_Dialect.enso b/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/Postgres/Postgres_Dialect.enso index f18073356328..b9f173005667 100644 --- a/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/Postgres/Postgres_Dialect.enso +++ b/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/Postgres/Postgres_Dialect.enso @@ -325,6 +325,10 @@ type Postgres_Dialect _ = value_type False + ## PRIVATE + generate_column_for_select self base_gen expr:(SQL_Expression | Order_Descriptor | Query) name:Text -> SQL_Builder = + base_gen.default_generate_column self expr name + ## PRIVATE ensure_query_has_no_holes : JDBC_Connection -> Text -> Nothing ! Illegal_Argument ensure_query_has_no_holes self jdbc:JDBC_Connection raw_sql:Text = diff --git a/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/SQLite/SQLite_Dialect.enso b/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/SQLite/SQLite_Dialect.enso index ab8aa5aa1524..6d455230d7ec 100644 --- a/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/SQLite/SQLite_Dialect.enso +++ b/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/SQLite/SQLite_Dialect.enso @@ -328,6 +328,10 @@ type SQLite_Dialect _ = value_type False + ## PRIVATE + generate_column_for_select self base_gen expr:(SQL_Expression | Order_Descriptor | Query) name:Text -> SQL_Builder = + base_gen.default_generate_column self expr name + ## PRIVATE ensure_query_has_no_holes : JDBC_Connection -> Text -> Nothing ! Illegal_Argument ensure_query_has_no_holes self jdbc:JDBC_Connection raw_sql:Text = diff --git a/distribution/lib/Standard/Microsoft/0.0.0-dev/src/Internal/SQLServer_Dialect.enso b/distribution/lib/Standard/Microsoft/0.0.0-dev/src/Internal/SQLServer_Dialect.enso index bcddddb11405..06c5d6309f08 100644 --- a/distribution/lib/Standard/Microsoft/0.0.0-dev/src/Internal/SQLServer_Dialect.enso +++ b/distribution/lib/Standard/Microsoft/0.0.0-dev/src/Internal/SQLServer_Dialect.enso @@ -102,6 +102,12 @@ type SQLServer_Dialect wrap_identifier self identifier = Base_Generator.wrap_in_quotes identifier + ## PRIVATE + Generates a SQL expression for a table literal. + make_table_literal : Vector (Vector Text) -> Vector Text -> Text -> SQL_Builder + make_table_literal self vecs column_names as_name = + Base_Generator.default_make_table_literal self.wrap_identifier vecs column_names as_name + ## PRIVATE Prepares an ordering descriptor. @@ -227,6 +233,7 @@ type SQLServer_Dialect is_feature_supported self feature:Feature -> Boolean = case feature of Feature.Select_Columns -> True + Feature.Sort -> True _ -> False ## PRIVATE @@ -302,13 +309,81 @@ type SQLServer_Dialect if raw_sql.contains "#" . not then jdbc.ensure_query_has_no_holes raw_sql + ## PRIVATE + Returns a pair of a SQL_Builder for the given expression and a vector of columns + that have been used in the expression and need to be checked for nulls. + SQL Server needs special handling commpared to ther databases as it does not have a + boolean data type. + This means that you can write + SELECT * FROM MyTable WHERE [Column1] > [Column2] + but you cannot write + SELECT [Column1] > [Column2] FROM MyTable + to write the second query you need to write + SELECT CASE WHEN [Column1] IS NULL OR [Column2] IS NULL WHEN [Column1] > [Column2] THEN 1 ELSE 0 END FROM MyTable + The below function collects all of the fields which are needed to be checked for nulls returning them in a vector + as the second element of the pair. + The first element of the pair is the SQL_Builder for the expression. + generate_expression self base_gen expr = case expr of + SQL_Expression.Column _ _ -> + wrapped_name = base_gen.generate_expression self expr + pair wrapped_name [wrapped_name] + SQL_Expression.Constant value -> + wrapped = case value of + Nothing -> SQL_Builder.code "NULL" + _ -> SQL_Builder.interpolation value + pair wrapped [wrapped] + SQL_Expression.Literal value -> + wrapped = SQL_Builder.code value + pair wrapped [wrapped] + SQL_Expression.Text_Literal _ -> + wrapped_literal = base_gen.generate_expression self expr + pair wrapped_literal [] + SQL_Expression.Operation kind arguments metadata -> + op = self.dialect_operations.operations_dict.get kind (Error.throw <| Unsupported_Database_Operation.Error kind) + parsed_args_and_null_checks = arguments.map (c -> self.generate_expression base_gen c) + parsed_args = parsed_args_and_null_checks.map .first + null_checks = parsed_args_and_null_checks.map .second . flatten + + expr_result = case kind of + ## In the case that we actually want to check for null then we need to generate the null + check sql for all the columns that have been used up to this point and that + becomes the expression. + "IS_NULL" -> _generate_null_check_sql_builder null_checks + _ -> + result = op parsed_args + # If the function expects more arguments, we pass the metadata as the last argument. + case result of + _ : Function -> result metadata + _ -> result + null_checks_result = if kind == "IS_NULL" then [] else null_checks + + + pair expr_result null_checks_result + query : Query -> pair (base_gen.generate_sub_query self query) [] + + ## PRIVATE + generate_column_for_select self base_gen expr:(SQL_Expression | Order_Descriptor | Query) name:Text -> SQL_Builder = + expr_null_checks_pair = self.generate_expression base_gen expr + base_expr = expr_null_checks_pair.first + built_expr = case expr of + SQL_Expression.Operation _ _ _ -> + null_check = if expr_null_checks_pair.second.length == 0 then "" else + SQL_Builder.code "WHEN " ++ _generate_null_check_sql_builder expr_null_checks_pair.second ++ " THEN NULL " + SQL_Builder.code "CASE " ++ null_check ++ "WHEN " ++ base_expr ++ " THEN 1 ELSE 0 END" + _ -> base_expr + built_expr ++ Base_Generator.alias self name + +## PRIVATE +private _generate_null_check_sql_builder null_checks:Vector -> SQL_Builder = + (null_checks.map it->(it.paren ++ " IS NULL ")) . reduce acc-> i-> acc ++ "OR " ++ i + ## PRIVATE make_dialect_operations = cases = [["LOWER", Base_Generator.make_function "LOWER"], ["UPPER", Base_Generator.make_function "UPPER"]] text = [starts_with, contains, ends_with, agg_shortest, agg_longest, make_case_sensitive, ["REPLACE", replace], left, right]+concat_ops+cases+trim_ops counts = [agg_count_is_null, agg_count_empty, agg_count_not_empty, ["COUNT_DISTINCT", agg_count_distinct], ["COUNT_DISTINCT_INCLUDE_NULL", agg_count_distinct_include_null]] arith_extensions = [is_nan, is_inf, is_finite, floating_point_div, mod_op, decimal_div, decimal_mod, ["ROW_MIN", Base_Generator.make_function "LEAST"], ["ROW_MAX", Base_Generator.make_function "GREATEST"]] - bool = [bool_or] + bool = [bool_or, bool_not] eq = lift_binary_op "==" make_equals compare = [eq] @@ -319,7 +394,8 @@ make_dialect_operations = special_overrides = [is_null] other = [["RUNTIME_ERROR", make_runtime_error_op]] my_mappings = text + counts + stats + first_last_aggregators + arith_extensions + bool + compare + date_ops + special_overrides + other - Base_Generator.base_dialect_operations . extend_with my_mappings + base = Base_Generator.base_dialect_operations . extend_with my_mappings + Base_Generator.Dialect_Operations.Value (base.operations_dict.remove "IS_IN") ## PRIVATE is_null = Base_Generator.lift_unary_op "IS_NULL" arg-> @@ -500,6 +576,10 @@ is_finite = Base_Generator.lift_unary_op "IS_FINITE" arg-> bool_or = Base_Generator.lift_unary_op "BOOL_OR" arg-> SQL_Builder.code "bool_or(" ++ arg ++ ")" +## PRIVATE +bool_not = Base_Generator.lift_unary_op "NOT" arg-> + SQL_Builder.code "(" ++ arg ++ ")=0" + ## PRIVATE floating_point_div = Base_Generator.lift_binary_op "/" x-> y-> SQL_Builder.code "CAST(" ++ x ++ " AS double precision) / CAST(" ++ y ++ " AS double precision)" diff --git a/distribution/lib/Standard/Microsoft/0.0.0-dev/src/Internal/SQLServer_Type_Mapping.enso b/distribution/lib/Standard/Microsoft/0.0.0-dev/src/Internal/SQLServer_Type_Mapping.enso index 1d654a2577c8..244f92b8f26b 100644 --- a/distribution/lib/Standard/Microsoft/0.0.0-dev/src/Internal/SQLServer_Type_Mapping.enso +++ b/distribution/lib/Standard/Microsoft/0.0.0-dev/src/Internal/SQLServer_Type_Mapping.enso @@ -108,7 +108,7 @@ type SQLServer_Type_Mapping ## PRIVATE The SQLServer_Type_Mapping always relies on the return type determined by - the database backend. + the database backend except for boolean types. infer_return_type : (SQL_Expression -> SQL_Type_Reference) -> Text -> Vector -> SQL_Expression -> SQL_Type_Reference infer_return_type infer_from_database_callback op_name arguments expression = case operations_dict.contains_key op_name of @@ -159,7 +159,7 @@ on_unknown_type sql_type = ## PRIVATE Maps operation names to functions that infer its result type. operations_dict : Dictionary Text (Vector -> SQL_Type) -operations_dict = Dictionary.from_vector [["IS_NULL", const (SQL_Type.Value Types.BIT "BIT")],["==", const (SQL_Type.Value Types.BIT "BIT")]] +operations_dict = Dictionary.from_vector [["IS_NULL", const (SQL_Type.Value Types.BIT "BIT")],["==", const (SQL_Type.Value Types.BIT "BIT")],["NOT", const (SQL_Type.Value Types.BIT "BIT")],["BETWEEN", const (SQL_Type.Value Types.BIT "BIT")]] ## PRIVATE This is the maximum size that JDBC driver reports for 'unbounded' types in diff --git a/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Internal/Snowflake_Dialect.enso b/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Internal/Snowflake_Dialect.enso index 4c6085933f57..c0a154a69f0e 100644 --- a/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Internal/Snowflake_Dialect.enso +++ b/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Internal/Snowflake_Dialect.enso @@ -308,6 +308,10 @@ type Snowflake_Dialect Value_Type.Date_Time _ -> True _ -> False + ## PRIVATE + generate_column_for_select self base_gen expr:(SQL_Expression | Order_Descriptor | Query) name:Text -> SQL_Builder = + base_gen.default_generate_column self expr name + ## PRIVATE ensure_query_has_no_holes : JDBC_Connection -> Text -> Nothing ! Illegal_Argument ensure_query_has_no_holes self jdbc:JDBC_Connection raw_sql:Text = diff --git a/test/AWS_Tests/src/Redshift_Spec.enso b/test/AWS_Tests/src/Redshift_Spec.enso index abb991c696e1..fa6780c2d1cf 100644 --- a/test/AWS_Tests/src/Redshift_Spec.enso +++ b/test/AWS_Tests/src/Redshift_Spec.enso @@ -85,9 +85,10 @@ add_database_specs suite_builder create_connection_fn = (agg_in_memory_table.take (..First 0)).select_into_database_table default_connection.get (Name_Generator.random_name "Agg_Empty") primary_key=Nothing temporary=True is_feature_supported_fn feature:Feature = default_connection.get.dialect.is_feature_supported feature + is_operation_supported_fn operation:Text = default_connection.get.dialect.is_operation_supported operation flagged_fn = default_connection.get.dialect.flagged - setup = Common_Table_Operations.Main.Test_Setup.Config prefix agg_table_fn empty_agg_table_fn table_builder materialize is_database=True test_selection=common_selection aggregate_test_selection=aggregate_selection create_connection_func=create_connection_fn light_table_builder=light_table_builder is_feature_supported=is_feature_supported_fn flagged=flagged_fn + setup = Common_Table_Operations.Main.Test_Setup.Config prefix agg_table_fn empty_agg_table_fn table_builder materialize is_database=True test_selection=common_selection aggregate_test_selection=aggregate_selection create_connection_func=create_connection_fn light_table_builder=light_table_builder is_feature_supported=is_feature_supported_fn flagged=flagged_fn is_operation_supported=is_operation_supported_fn Common_Spec.add_specs suite_builder prefix create_connection_fn default_connection setup add_redshift_specific_specs suite_builder create_connection_fn setup diff --git a/test/Microsoft_Tests/src/SQLServer_Spec.enso b/test/Microsoft_Tests/src/SQLServer_Spec.enso index 3e387857660a..2db316f15544 100644 --- a/test/Microsoft_Tests/src/SQLServer_Spec.enso +++ b/test/Microsoft_Tests/src/SQLServer_Spec.enso @@ -207,9 +207,10 @@ add_sqlserver_specs suite_builder create_connection_fn = (agg_in_memory_table.take (..First 0)).select_into_database_table default_connection.get (Name_Generator.random_name "Agg_Empty") primary_key=Nothing temporary=True is_feature_supported_fn feature:Feature = default_connection.get.dialect.is_feature_supported feature + is_operation_supported_fn operation:Text = default_connection.get.dialect.is_operation_supported operation flagged_fn = default_connection.get.dialect.flagged - setup = Common_Table_Operations.Main.Test_Setup.Config prefix agg_table_fn empty_agg_table_fn table_builder materialize is_database=True test_selection=common_selection aggregate_test_selection=aggregate_selection create_connection_func=create_connection_fn light_table_builder=light_table_builder is_feature_supported=is_feature_supported_fn flagged=flagged_fn + setup = Common_Table_Operations.Main.Test_Setup.Config prefix agg_table_fn empty_agg_table_fn table_builder materialize is_database=True test_selection=common_selection aggregate_test_selection=aggregate_selection create_connection_func=create_connection_fn light_table_builder=light_table_builder is_feature_supported=is_feature_supported_fn flagged=flagged_fn is_operation_supported=is_operation_supported_fn Common_Spec.add_specs suite_builder prefix create_connection_fn default_connection setup Common_Table_Operations.Main.add_specs suite_builder setup diff --git a/test/Snowflake_Tests/src/Snowflake_Spec.enso b/test/Snowflake_Tests/src/Snowflake_Spec.enso index 828873d1b581..f06909fe7f7f 100644 --- a/test/Snowflake_Tests/src/Snowflake_Spec.enso +++ b/test/Snowflake_Tests/src/Snowflake_Spec.enso @@ -547,9 +547,10 @@ add_snowflake_specs suite_builder create_connection_fn db_name = (agg_in_memory_table.take (..First 0)).select_into_database_table default_connection.get (Name_Generator.random_name "Agg_Empty") primary_key=Nothing temporary=True is_feature_supported_fn feature:Feature = default_connection.get.dialect.is_feature_supported feature + is_operation_supported_fn operation:Text = default_connection.get.dialect.is_operation_supported operation flagged_fn = default_connection.get.dialect.flagged - setup = Common_Table_Operations.Main.Test_Setup.Config prefix agg_table_fn empty_agg_table_fn table_builder materialize is_database=True test_selection=common_selection aggregate_test_selection=aggregate_selection create_connection_func=create_connection_fn light_table_builder=light_table_builder is_integer_type=is_snowflake_integer is_feature_supported=is_feature_supported_fn flagged=flagged_fn + setup = Common_Table_Operations.Main.Test_Setup.Config prefix agg_table_fn empty_agg_table_fn table_builder materialize is_database=True test_selection=common_selection aggregate_test_selection=aggregate_selection create_connection_func=create_connection_fn light_table_builder=light_table_builder is_integer_type=is_snowflake_integer is_feature_supported=is_feature_supported_fn flagged=flagged_fn is_operation_supported=is_operation_supported_fn Common_Spec.add_specs suite_builder prefix create_connection_fn default_connection setup snowflake_specific_spec suite_builder default_connection db_name setup diff --git a/test/Table_Tests/src/Common_Table_Operations/Add_Row_Number_Spec.enso b/test/Table_Tests/src/Common_Table_Operations/Add_Row_Number_Spec.enso index 5d63ec17479c..5889ec920c65 100644 --- a/test/Table_Tests/src/Common_Table_Operations/Add_Row_Number_Spec.enso +++ b/test/Table_Tests/src/Common_Table_Operations/Add_Row_Number_Spec.enso @@ -30,7 +30,7 @@ type Data self.connection.close add_specs suite_builder setup = - if setup.is_feature_supported Feature.Filter then (add_row_number_specs suite_builder setup) else + if setup.is_feature_supported Feature.Add_Row_Number then (add_row_number_specs suite_builder setup) else suite_builder.group setup.prefix+"Table.add_row_number" group_builder-> group_builder.specify "add_row_number should report unsupported" <| table_builder = setup.light_table_builder diff --git a/test/Table_Tests/src/Common_Table_Operations/Main.enso b/test/Table_Tests/src/Common_Table_Operations/Main.enso index bbcab8295252..c3cc206738bd 100644 --- a/test/Table_Tests/src/Common_Table_Operations/Main.enso +++ b/test/Table_Tests/src/Common_Table_Operations/Main.enso @@ -73,7 +73,7 @@ type Test_Setup boolean indicating if the feature is supported by the backend. - flagged: A function that takes a `Dialect_Flag` and returns a boolean indicating if the flag is set for the backend. - Config prefix table_fn empty_table_fn (table_builder : (Vector Any -> (Any|Nothing)) -> Any) materialize is_database test_selection aggregate_test_selection create_connection_func light_table_builder is_integer_type=(.is_integer) is_feature_supported flagged + Config prefix table_fn empty_table_fn (table_builder : (Vector Any -> (Any|Nothing)) -> Any) materialize is_database test_selection aggregate_test_selection create_connection_func light_table_builder is_integer_type=(.is_integer) is_feature_supported flagged is_operation_supported ## Specifies if the given Table backend supports custom Enso types. diff --git a/test/Table_Tests/src/Common_Table_Operations/Nothing_Spec.enso b/test/Table_Tests/src/Common_Table_Operations/Nothing_Spec.enso index 3e19873d8b4b..9260e3f20e52 100644 --- a/test/Table_Tests/src/Common_Table_Operations/Nothing_Spec.enso +++ b/test/Table_Tests/src/Common_Table_Operations/Nothing_Spec.enso @@ -131,7 +131,7 @@ add_nothing_specs suite_builder setup = table = setup.light_table_builder [["x", [True, False, Nothing]]] table.at "x" . not . to_vector . should_equal [False, True, Nothing] - suite_builder.group prefix+"(Nothing_Spec) is_in" group_builder-> + if setup.is_operation_supported "IS_IN" then suite_builder.group prefix+"(Nothing_Spec) is_in" group_builder-> values_with_nothing.map triple-> value = triple.at 0 other_value = triple.at 1 @@ -176,7 +176,7 @@ add_nothing_specs suite_builder setup = table = table_builder_typed [["x", [value, Nothing]], ["y", [other_value, Nothing]], ["z", [value, other_value]], ["n", [Nothing, Nothing]]] value_type table.at "x" . is_in [] . to_vector . should_equal [False, False] - if setup.test_selection.run_advanced_edge_case_tests then suite_builder.group prefix+"(Nothing_Spec) is_in: Boolean+Nothing edge cases" group_builder-> + if setup.test_selection.run_advanced_edge_case_tests && setup.is_operation_supported "IS_IN" then suite_builder.group prefix+"(Nothing_Spec) is_in: Boolean+Nothing edge cases" group_builder-> make_containing_values had_null had_true had_false = null_maybe = if had_null then [Nothing] else [] true_maybe = if had_true then [True] else [] @@ -222,7 +222,7 @@ add_nothing_specs suite_builder setup = c.to_vector . should_equal [output] - group_builder.specify "Boolean is_in: edge cases (Column), "+negation_desc+" "+cs.to_text <| + if setup.is_feature_supported Feature.Distinct then group_builder.specify "Boolean is_in: edge cases (Column), "+negation_desc+" "+cs.to_text <| input_column = transform_input (Vector.fill containing_values.length input) t = table_builder_typed [["input", input_column], ["containing", transform_argument containing_values]] Value_Type.Boolean expected_output = if input_column.is_empty then [] else [output] @@ -232,7 +232,7 @@ add_nothing_specs suite_builder setup = c.to_vector . length . should_equal input_column.length c.to_vector.distinct . should_equal expected_output - suite_builder.group prefix+"(Nothing_Spec) distinct" group_builder-> + if setup.is_feature_supported Feature.Distinct then suite_builder.group prefix+"(Nothing_Spec) distinct" group_builder-> values_without_nothing.map triple-> value = triple.at 0 other_value = triple.at 1 diff --git a/test/Table_Tests/src/Database/Postgres_Spec.enso b/test/Table_Tests/src/Database/Postgres_Spec.enso index 8452dcc5f4c6..6df0b1707266 100644 --- a/test/Table_Tests/src/Database/Postgres_Spec.enso +++ b/test/Table_Tests/src/Database/Postgres_Spec.enso @@ -737,9 +737,10 @@ add_postgres_specs suite_builder create_connection_fn db_name = (agg_in_memory_table.take (..First 0)).select_into_database_table default_connection.get (Name_Generator.random_name "Agg_Empty") primary_key=Nothing temporary=True is_feature_supported_fn feature:Feature = default_connection.get.dialect.is_feature_supported feature + is_operation_supported_fn operation:Text = default_connection.get.dialect.is_operation_supported operation flagged_fn = default_connection.get.dialect.flagged - setup = Common_Table_Operations.Main.Test_Setup.Config prefix agg_table_fn empty_agg_table_fn table_builder materialize is_database=True test_selection=common_selection aggregate_test_selection=aggregate_selection create_connection_func=create_connection_fn light_table_builder=light_table_builder is_feature_supported=is_feature_supported_fn flagged=flagged_fn + setup = Common_Table_Operations.Main.Test_Setup.Config prefix agg_table_fn empty_agg_table_fn table_builder materialize is_database=True test_selection=common_selection aggregate_test_selection=aggregate_selection create_connection_func=create_connection_fn light_table_builder=light_table_builder is_feature_supported=is_feature_supported_fn flagged=flagged_fn is_operation_supported=is_operation_supported_fn Common_Spec.add_specs suite_builder prefix create_connection_fn default_connection setup postgres_specific_spec suite_builder create_connection_fn db_name setup diff --git a/test/Table_Tests/src/Database/SQLite_Spec.enso b/test/Table_Tests/src/Database/SQLite_Spec.enso index e4e57f1e294a..e658f5d91737 100644 --- a/test/Table_Tests/src/Database/SQLite_Spec.enso +++ b/test/Table_Tests/src/Database/SQLite_Spec.enso @@ -352,9 +352,10 @@ sqlite_spec suite_builder prefix create_connection_func persistent_connector = (agg_in_memory_table.take (..First 0)).select_into_database_table default_connection.get (Name_Generator.random_name "Agg_Empty") primary_key=Nothing temporary=True is_feature_supported_fn feature:Feature = default_connection.get.dialect.is_feature_supported feature + is_operation_supported_fn operation:Text = default_connection.get.dialect.is_operation_supported operation flagged_fn = default_connection.get.dialect.flagged - setup = Common_Table_Operations.Main.Test_Setup.Config prefix agg_table_fn empty_agg_table_fn table_builder materialize is_database=True test_selection=common_selection aggregate_test_selection=aggregate_selection create_connection_func=create_connection_func light_table_builder=light_table_builder is_feature_supported=is_feature_supported_fn flagged=flagged_fn + setup = Common_Table_Operations.Main.Test_Setup.Config prefix agg_table_fn empty_agg_table_fn table_builder materialize is_database=True test_selection=common_selection aggregate_test_selection=aggregate_selection create_connection_func=create_connection_func light_table_builder=light_table_builder is_feature_supported=is_feature_supported_fn flagged=flagged_fn is_operation_supported=is_operation_supported_fn Common_Spec.add_specs suite_builder prefix create_connection_func default_connection setup sqlite_specific_spec suite_builder prefix create_connection_func setup diff --git a/test/Table_Tests/src/In_Memory/Common_Spec.enso b/test/Table_Tests/src/In_Memory/Common_Spec.enso index b92b8480e852..6dbf3c3df04a 100644 --- a/test/Table_Tests/src/In_Memory/Common_Spec.enso +++ b/test/Table_Tests/src/In_Memory/Common_Spec.enso @@ -37,11 +37,13 @@ in_memory_setup = Dummy_Connection.Value is_feature_supported_fn _ = True + is_operation_supported_fn _ = + True flagged_fn flag:Dialect_Flag = case flag of Dialect_Flag.Supports_Case_Sensitive_Columns -> True - Common_Table_Operations.Main.Test_Setup.Config "[In-Memory] " agg_table_fn empty_table_fn table_builder materialize is_database=False test_selection=selection aggregate_test_selection=aggregate_selection create_connection_func=create_connection_func light_table_builder=light_table_builder is_feature_supported=is_feature_supported_fn flagged=flagged_fn + Common_Table_Operations.Main.Test_Setup.Config "[In-Memory] " agg_table_fn empty_table_fn table_builder materialize is_database=False test_selection=selection aggregate_test_selection=aggregate_selection create_connection_func=create_connection_func light_table_builder=light_table_builder is_feature_supported=is_feature_supported_fn flagged=flagged_fn is_operation_supported=is_operation_supported_fn add_specs suite_builder = Common_Table_Operations.Main.add_specs suite_builder in_memory_setup From 08fd9213e4ec34bec76550e503b70fee9cd4bca5 Mon Sep 17 00:00:00 2001 From: Adam Obuchowicz Date: Fri, 25 Oct 2024 14:26:21 +0200 Subject: [PATCH 02/13] Table Input Widget: Add column with plus (#11388) Fixes #10863 (The `Column #3` in a screenshot below is just created and actually existing). ![image](https://github.com/user-attachments/assets/995d1a85-eb04-4b8d-bf23-b47ea61185e4) --- CHANGELOG.md | 3 + app/gui/e2e/project-view/rightPanel.spec.ts | 11 +- .../project-view/tableVisualisation.spec.ts | 1 + app/gui/e2e/project-view/widgets.spec.ts | 29 ++-- .../GraphEditor/widgets/WidgetTableEditor.vue | 18 ++- .../widgets/WidgetTableEditor/TableHeader.vue | 75 +++++++--- .../__tests__/tableNewArgument.test.ts | 132 +++++++++--------- .../WidgetTableEditor/tableNewArgument.ts | 64 +++++---- .../components/TooltipTrigger.vue | 1 - app/gui/src/project-view/providers/index.ts | 5 +- 10 files changed, 203 insertions(+), 136 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b397ec7fa72d..cbfc042aba21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,12 +9,15 @@ - [Copying and pasting in Table Editor Widget now works properly][11332] - [Fix invisible selection in Table Input Widget][11358] - [Enable cloud file browser in local projects][11383] +- [Changed the way of adding new column in Table Input Widget][11388]. The + "virtual column" is replaced with an explicit (+) button. [11151]: https://github.com/enso-org/enso/pull/11151 [11271]: https://github.com/enso-org/enso/pull/11271 [11332]: https://github.com/enso-org/enso/pull/11332 [11358]: https://github.com/enso-org/enso/pull/11358 [11383]: https://github.com/enso-org/enso/pull/11383 +[11388]: https://github.com/enso-org/enso/pull/11388 #### Enso Standard Library diff --git a/app/gui/e2e/project-view/rightPanel.spec.ts b/app/gui/e2e/project-view/rightPanel.spec.ts index d8cc94aa4649..474231e5ecae 100644 --- a/app/gui/e2e/project-view/rightPanel.spec.ts +++ b/app/gui/e2e/project-view/rightPanel.spec.ts @@ -26,15 +26,16 @@ test('Doc panel focus (regression #10471)', async ({ page }) => { await page.keyboard.press(`${CONTROL_KEY}+D`) await page.keyboard.press(`${CONTROL_KEY}+\``) await expect(locate.rightDock(page)).toBeVisible() - await expect(locate.bottomDock(page)).toBeVisible() + const codeEditor = page.locator('.CodeEditor') + await expect(codeEditor).toBeVisible() // Focus code editor. - await locate.bottomDock(page).click() + await codeEditor.click() await page.evaluate(() => { - const codeEditor = (window as any).__codeEditorApi - const docStart = codeEditor.indexOf('The main method') - codeEditor.placeCursor(docStart + 8) + const codeEditorApi = (window as any).__codeEditorApi + const docStart = codeEditorApi.indexOf('The main method') + codeEditorApi.placeCursor(docStart + 8) }) await page.keyboard.press('Space') await page.keyboard.press('T') diff --git a/app/gui/e2e/project-view/tableVisualisation.spec.ts b/app/gui/e2e/project-view/tableVisualisation.spec.ts index 168ff37cc994..73fd06f10f4d 100644 --- a/app/gui/e2e/project-view/tableVisualisation.spec.ts +++ b/app/gui/e2e/project-view/tableVisualisation.spec.ts @@ -58,6 +58,7 @@ test('Copy from Table Visualization', async ({ page, context }) => { const node = await actions.createTableNode(page) const widget = node.locator('.WidgetTableEditor') await expect(widget).toBeVisible() + await widget.getByRole('button', { name: 'Add new column' }).click() await widget.locator('.ag-cell', { hasNotText: /0/ }).first().click() await page.keyboard.press('Control+V') await expect(widget.locator('.ag-cell')).toHaveText([ diff --git a/app/gui/e2e/project-view/widgets.spec.ts b/app/gui/e2e/project-view/widgets.spec.ts index 8c33e473f748..21a1b2697ea3 100644 --- a/app/gui/e2e/project-view/widgets.spec.ts +++ b/app/gui/e2e/project-view/widgets.spec.ts @@ -544,27 +544,32 @@ test('Table widget', async ({ page }) => { const node = await actions.createTableNode(page) const widget = node.locator('.WidgetTableEditor') await expect(widget).toBeVisible() - await expect(widget.locator('.ag-header-cell-text')).toHaveText(['#', 'New Column']) - await expect(widget.locator('.ag-header-cell-text', { hasText: 'New Column' })).toHaveClass( - /(?<=^| )virtualColumn(?=$| )/, - ) - // There are two cells, one with row number, second allowing creating first row and column - await expect(widget.locator('.ag-cell')).toHaveCount(2) + await expect(widget.locator('.ag-header-cell-text')).toHaveText(['#']) + await expect(widget.getByRole('button', { name: 'Add new column' })).toExist() + await expect(widget.locator('.ag-cell')).toHaveText(['0', '']) + + // Create first column + await widget.getByRole('button', { name: 'Add new column' }).click() + await expect(widget.locator('.ag-header-cell-text')).toHaveText(['#', 'Column #1']) + await expect(widget.locator('.ag-cell')).toHaveText(['0', '', '']) // Putting first value - await widget.locator('.ag-cell', { hasNotText: '0' }).click() + await widget.locator('.ag-cell', { hasNotText: '0' }).first().click() await page.keyboard.type('Value') await page.keyboard.press('Enter') - // There will be new blank column and new blank row allowing adding new columns and rows - // (so 4 cells in total) - await expect(widget.locator('.ag-header-cell-text')).toHaveText(['#', 'Column #0', 'New Column']) + // There will be new blank row allowing adding new rows. await expect(widget.locator('.ag-cell')).toHaveText(['0', 'Value', '', '1', '', '']) // Renaming column - await widget.locator('.ag-header-cell-text', { hasText: 'Column #0' }).first().click() + await widget.locator('.ag-header-cell-text', { hasText: 'Column #1' }).first().click() await page.keyboard.type('Header') await page.keyboard.press('Enter') - await expect(widget.locator('.ag-header-cell-text')).toHaveText(['#', 'Header', 'New Column']) + await expect(widget.locator('.ag-header-cell-text')).toHaveText(['#', 'Header']) + + // Adding next column + await widget.getByRole('button', { name: 'Add new column' }).click() + await expect(widget.locator('.ag-header-cell-text')).toHaveText(['#', 'Header', 'Column #2']) + await expect(widget.locator('.ag-cell')).toHaveText(['0', 'Value', '', '', '1', '', '', '']) // Switching edit between cells and headers - check we will never edit two things at once. await expect(widget.locator('.ag-text-field-input')).toHaveCount(0) diff --git a/app/gui/src/project-view/components/GraphEditor/widgets/WidgetTableEditor.vue b/app/gui/src/project-view/components/GraphEditor/widgets/WidgetTableEditor.vue index 78dcedc52ca1..da5a17f3f257 100644 --- a/app/gui/src/project-view/components/GraphEditor/widgets/WidgetTableEditor.vue +++ b/app/gui/src/project-view/components/GraphEditor/widgets/WidgetTableEditor.vue @@ -9,6 +9,7 @@ import { import ResizeHandles from '@/components/ResizeHandles.vue' import AgGridTableView from '@/components/shared/AgGridTableView.vue' import { injectGraphNavigator } from '@/providers/graphNavigator' +import { useTooltipRegistry } from '@/providers/tooltipState' import { Score, defineWidget, widgetProps } from '@/providers/widgetRegistry' import { WidgetEditHandler } from '@/providers/widgetRegistry/editHandler' import { useGraphStore } from '@/stores/graph' @@ -20,12 +21,13 @@ import '@ag-grid-community/styles/ag-theme-alpine.css' import type { CellEditingStartedEvent, CellEditingStoppedEvent, + ColDef, Column, ColumnMovedEvent, ProcessDataFromClipboardParams, RowDragEndEvent, } from 'ag-grid-enterprise' -import { computed, ref } from 'vue' +import { computed, markRaw, ref } from 'vue' import type { ComponentExposed } from 'vue-component-type-helpers' const props = defineProps(widgetProps(widgetDefinition)) @@ -178,14 +180,22 @@ function processDataFromClipboard({ data, api }: ProcessDataFromClipboardParams< // === Column Default Definition === -const defaultColDef = { +const tooltipRegistry = useTooltipRegistry() +const defaultColDef: ColDef = { editable: true, resizable: true, sortable: false, lockPinned: true, + menuTabs: ['generalMenuTab'], headerComponentParams: { - onHeaderEditingStarted: headerEditHandler.headerEditedInGrid.bind(headerEditHandler), - onHeaderEditingStopped: headerEditHandler.headerEditingStoppedInGrid.bind(headerEditHandler), + // TODO[ao]: we mark raw, because otherwise any change _inside_ tooltipRegistry causes the grid + // to be refreshed. Technically, shallowReactive should work here, but it does not, + // I don't know why + tooltipRegistry: markRaw(tooltipRegistry), + editHandlers: { + onHeaderEditingStarted: headerEditHandler.headerEditedInGrid.bind(headerEditHandler), + onHeaderEditingStopped: headerEditHandler.headerEditingStoppedInGrid.bind(headerEditHandler), + }, }, } diff --git a/app/gui/src/project-view/components/GraphEditor/widgets/WidgetTableEditor/TableHeader.vue b/app/gui/src/project-view/components/GraphEditor/widgets/WidgetTableEditor/TableHeader.vue index 8ffe338fefa4..710a48b8d9c3 100644 --- a/app/gui/src/project-view/components/GraphEditor/widgets/WidgetTableEditor/TableHeader.vue +++ b/app/gui/src/project-view/components/GraphEditor/widgets/WidgetTableEditor/TableHeader.vue @@ -1,22 +1,39 @@ @@ -25,17 +42,23 @@ const props = defineProps<{ params: IHeaderParams & HeaderParams }>() +/** Re-provide tooltipRegistry. See `tooltipRegistry` docs in {@link HeaderParams} */ +provideTooltipRegistry.provideConstructed(props.params.tooltipRegistry) + const editing = ref(false) const inputElement = ref() +const editHandlers = computed(() => + props.params.type === 'astColumn' ? props.params.editHandlers : undefined, +) watch(editing, (newVal) => { if (newVal) { - props.params.onHeaderEditingStarted?.((cancel: boolean) => { + editHandlers.value?.onHeaderEditingStarted?.((cancel: boolean) => { if (cancel) editing.value = false else acceptNewName() }) } else { - props.params.onHeaderEditingStopped?.() + editHandlers.value?.onHeaderEditingStopped?.() } }) @@ -48,33 +71,47 @@ watch(inputElement, (newVal, oldVal) => { }) function acceptNewName() { + if (editHandlers.value == null) { + console.error("Tried to accept header new name where it's not editable!") + return + } if (inputElement.value == null) { console.error('Tried to accept header new name without input element!') return } - props.params.nameSetter?.(inputElement.value.value) + editHandlers.value.nameSetter(inputElement.value.value) editing.value = false } -function onMouseClick() { - if (!editing.value && props.params.nameSetter != null) { +function onMouseClick(event: MouseEvent) { + if (!editing.value && props.params.type === 'astColumn') { editing.value = true + event.stopPropagation() } } function onMouseRightClick(event: MouseEvent) { if (!editing.value) { props.params.showColumnMenuAfterMouseClick(event) + event.preventDefault() + event.stopPropagation() } } - diff --git a/app/gui/src/project-view/components/ColorRing.vue b/app/gui/src/project-view/components/ColorRing.vue index e2c5ef0ce7bf..e46a944dc258 100644 --- a/app/gui/src/project-view/components/ColorRing.vue +++ b/app/gui/src/project-view/components/ColorRing.vue @@ -192,6 +192,10 @@ const triangleStyle = computed(() => cursor: crosshair; border-radius: var(--radius-full); animation: grow 0.1s forwards; + clip-path: path( + evenodd, + 'M0,52 A52,52 0,1,1 104,52 A52,52 0,1,1 0, 52 z m52,20 A20,20 0,1,1 52,32 20,20 0,1,1 52,72 z' + ); } @keyframes grow { from { diff --git a/app/gui/src/project-view/components/GraphEditor.vue b/app/gui/src/project-view/components/GraphEditor.vue index cfd57004804f..7c44470db199 100644 --- a/app/gui/src/project-view/components/GraphEditor.vue +++ b/app/gui/src/project-view/components/GraphEditor.vue @@ -483,6 +483,12 @@ const rightDockVisible = useSelectRef( showRightDock, ) +/** Show help panel if it is not visible. If it is visible, close the right dock. */ +function toggleRightDockHelpPanel() { + rightDockVisible.value = !rightDockVisible.value || rightDockDisplayedTab.value !== 'help' + rightDockDisplayedTab.value = 'help' +} + function editWithComponentBrowser(node: NodeId, cursorPos: number) { openComponentBrowser( { type: 'editNode', node, cursorPos }, @@ -722,6 +728,7 @@ const documentationEditorFullscreen = ref(false) @nodeOutputPortDoubleClick="handleNodeOutputPortDoubleClick" @nodeDoubleClick="(id) => stackNavigator.enterNode(id)" @createNodes="createNodesFromSource" + @toggleDocPanel="toggleRightDockHelpPanel" /> props.node.vis?.width ?? null) const visualizationHeight = computed(() => props.node.vis?.height ?? null) -const isVisualizationEnabled = computed(() => props.node.vis?.visible ?? false) +const isVisualizationEnabled = computed({ + get: () => props.node.vis?.visible ?? false, + set: (enabled) => { + emit('update:visualizationEnabled', enabled) + }, +}) const isVisualizationPreviewed = computed( () => keyboard.mod && outputHovered.value && !isVisualizationEnabled.value, ) @@ -457,15 +463,12 @@ watchEffect(() => { { @pointerenter="menuHovered = true" @pointerleave="menuHovered = false" @update:nodeColor="emit('setNodeColor', $event)" + @createNewNode="setSelected(), emit('createNodes', [{ commit: false, content: undefined }])" + @toggleDocPanel="emit('toggleDocPanel')" @click.capture="setSelected" /> () const projectStore = useProjectStore() @@ -76,6 +77,7 @@ const graphNodeSelections = shallowRef() @outputPortDoubleClick="(_event, port) => emit('nodeOutputPortDoubleClick', port)" @doubleClick="emit('nodeDoubleClick', id)" @createNodes="emit('createNodes', id, $event)" + @toggleDocPanel="emit('toggleDocPanel')" @setNodeColor="graphStore.overrideNodeColor(id, $event)" @update:edited="graphStore.setEditedNode(id, $event)" @update:rect="graphStore.updateNodeRect(id, $event)" From d16fd63525964225b862a3e76894ae328fa22b40 Mon Sep 17 00:00:00 2001 From: Dmitry Bushev Date: Mon, 28 Oct 2024 12:17:53 +0300 Subject: [PATCH 05/13] Reload insight script on file change (#11415) Changelog: - update: reload insight script when the file was edited --- .../org/enso/common/ContextInsightSetup.java | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/engine/common/src/main/java/org/enso/common/ContextInsightSetup.java b/engine/common/src/main/java/org/enso/common/ContextInsightSetup.java index 375240b878d1..563745a1f6a8 100644 --- a/engine/common/src/main/java/org/enso/common/ContextInsightSetup.java +++ b/engine/common/src/main/java/org/enso/common/ContextInsightSetup.java @@ -2,8 +2,13 @@ import java.io.File; import java.io.IOException; +import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardWatchEventKinds; +import java.nio.file.WatchKey; +import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.FileTime; import java.util.function.Function; import org.graalvm.polyglot.Context; import org.graalvm.polyglot.Source; @@ -53,6 +58,78 @@ private ContextInsightSetup(Context ctx, Path file) { this.insightFile = file; } + private static final class InsightFileWatcher implements Runnable { + + private final Path insightFilePath; + private final Runnable onChange; + + public InsightFileWatcher(Path insightFilePath, Runnable onChange) { + this.insightFilePath = insightFilePath; + this.onChange = onChange; + } + + @Override + public void run() { + try { + watch(); + } catch (IOException e) { + new IOException("Error watching the insight file", e).printStackTrace(); + } + } + + private void watch() throws IOException { + final Path insightDirectory = insightFilePath.getParent(); + final Path insightFileName = insightFilePath.getFileName(); + FileTime lastRecordedModifiedTime = FileTime.fromMillis(0); + + try (final var watchService = FileSystems.getDefault().newWatchService()) { + insightDirectory.register( + watchService, + StandardWatchEventKinds.ENTRY_CREATE, + StandardWatchEventKinds.ENTRY_MODIFY); + while (true) { + WatchKey watchKey; + try { + watchKey = watchService.take(); + } catch (InterruptedException ignored) { + return; + } + + boolean isModified = false; + for (var watchEvent : watchKey.pollEvents()) { + if (watchEvent.kind() == StandardWatchEventKinds.OVERFLOW) { + continue; + } + + final Path eventPath = (Path) watchEvent.context(); + if (eventPath.endsWith(insightFileName)) { + final Path insightFilePath = insightDirectory.resolve(eventPath); + try { + final BasicFileAttributes attributes = + Files.readAttributes(insightFilePath, BasicFileAttributes.class); + if (attributes.lastModifiedTime().compareTo(lastRecordedModifiedTime) > 0) { + lastRecordedModifiedTime = attributes.lastModifiedTime(); + isModified = true; + } + } catch (IOException ignored) { + continue; + } + } + } + + if (isModified) { + onChange.run(); + } + + final boolean isValid = watchKey.reset(); + if (!isValid) { + break; + } + } + } + } + } + /** * Configures the context if {@link #INSIGHT_PROP} property is specified. This support is * development only. It can be (and will be) removed in the future. @@ -72,6 +149,7 @@ static void configureContext(Context ctx) throws AssertionError { : "Cannot find " + insightFile + " specified via " + INSIGHT_PROP + " property"; ACTIVE = new ContextInsightSetup(ctx, insightFile.toPath()); ACTIVE.initialize(); + ACTIVE.startFileWatcherThread(); } catch (Error | Exception ex) { var ae = new AssertionError( @@ -99,4 +177,22 @@ private void initialize() throws IOException { var insight = (Function) instrument.lookup(Function.class); insightHandle = insight.apply(insightSrc); } + + private void reload() { + try { + if (insightHandle != null) { + insightHandle.close(); + } + initialize(); + } catch (Exception e) { + new Exception("Error reloading the insight script", e).printStackTrace(); + } + } + + private void startFileWatcherThread() { + final InsightFileWatcher insightFileWatcher = new InsightFileWatcher(insightFile, this::reload); + final Thread thread = new Thread(insightFileWatcher, "InsightFileWatcherThread"); + thread.setDaemon(true); + thread.start(); + } } From db0582d11cccee13be42f20c3935bc1c09bc5d76 Mon Sep 17 00:00:00 2001 From: Dmitry Bushev Date: Mon, 28 Oct 2024 12:41:46 +0300 Subject: [PATCH 06/13] Metadata should not depend on absolute text spans (#11390) close #11304 Changelog: - update: add `ide.snapshot` optional metadata field containing the source code of the file - update: `syncFileContents` method tries to repair the metadata spans when it detects that the source file was edited and the received code does not match the code stored in the `ide.snapshot` metadata field # Important Notes Tested in gui --- app/ydoc-server/package.json | 1 + app/ydoc-server/src/fileFormat.ts | 1 + app/ydoc-server/src/languageServerSession.ts | 56 ++++++- pnpm-lock.yaml | 147 ++++++++++--------- 4 files changed, 129 insertions(+), 76 deletions(-) diff --git a/app/ydoc-server/package.json b/app/ydoc-server/package.json index a4d51c9860be..6e0cbd86d988 100644 --- a/app/ydoc-server/package.json +++ b/app/ydoc-server/package.json @@ -25,6 +25,7 @@ "debug": "^4.3.6", "fast-diff": "^1.3.0", "isomorphic-ws": "^5.0.0", + "js-base64": "^3.7.7", "lib0": "^0.2.85", "y-protocols": "^1.0.5", "ydoc-shared": "workspace:*", diff --git a/app/ydoc-server/src/fileFormat.ts b/app/ydoc-server/src/fileFormat.ts index f6d24bf51e64..6cf6c8787479 100644 --- a/app/ydoc-server/src/fileFormat.ts +++ b/app/ydoc-server/src/fileFormat.ts @@ -42,6 +42,7 @@ export const ideMetadata = z .object({ node: z.record(z.string().uuid(), nodeMetadata), import: z.record(z.string(), importMetadata), + snapshot: z.string().optional(), }) .passthrough() .default(() => defaultMetadata().ide) diff --git a/app/ydoc-server/src/languageServerSession.ts b/app/ydoc-server/src/languageServerSession.ts index 1bbfb59be9e2..80f7f42cad8f 100644 --- a/app/ydoc-server/src/languageServerSession.ts +++ b/app/ydoc-server/src/languageServerSession.ts @@ -1,4 +1,5 @@ import createDebug from 'debug' +import { Base64 } from 'js-base64' import * as json from 'lib0/json' import * as map from 'lib0/map' import { ObservableV2 } from 'lib0/observable' @@ -483,6 +484,14 @@ class ModulePersistence extends ObservableV2<{ removed: () => void }> { } } + private static encodeCodeSnapshot(code: string): string { + return Base64.encode(code) + } + + private static decodeCodeSnapshot(snapshot: string): string { + return Base64.decode(snapshot) + } + private sendLsUpdate( synced: EnsoFileParts, newCode: string | undefined, @@ -491,17 +500,24 @@ class ModulePersistence extends ObservableV2<{ removed: () => void }> { ) { if (this.syncedContent == null || this.syncedVersion == null) return - const code = newCode ?? synced.code + const newSnapshot = newCode && { + snapshot: ModulePersistence.encodeCodeSnapshot(newCode), + } const newMetadataJson = newMetadata && json.stringify({ ...this.syncedMeta, - ide: { ...this.syncedMeta.ide, node: newMetadata }, + ide: { + ...this.syncedMeta.ide, + ...newSnapshot, + node: newMetadata, + }, }) const idMapToPersist = (newIdMap || newMetadata) && ModulePersistence.getIdMapToPersist(newIdMap, newMetadata ?? this.syncedMeta.ide.node) const newIdMapToPersistJson = idMapToPersist && serializeIdMap(idMapToPersist) + const code = newCode ?? synced.code const newContent = combineFileParts({ code, idMapJson: newIdMapToPersistJson ?? synced.idMapJson ?? '[]', @@ -510,7 +526,7 @@ class ModulePersistence extends ObservableV2<{ removed: () => void }> { const edits: TextEdit[] = [] if (newCode) edits.push(...applyDiffAsTextEdits(0, synced.code, newCode)) - if (newIdMap || newMetadata) { + if (newIdMap || newMetadata || newSnapshot) { const oldMetaContent = this.syncedContent.slice(synced.code.length) const metaContent = newContent.slice(code.length) const metaStartLine = (code.match(/\n/g) ?? []).length @@ -569,6 +585,7 @@ class ModulePersistence extends ObservableV2<{ removed: () => void }> { const nodeMeta = Object.entries(metadata.ide.node) let parsedSpans + let parsedIdMap const syncModule = new Ast.MutableModule(this.doc.ydoc) if (code !== this.syncedCode) { const syncRoot = syncModule.root() @@ -579,16 +596,33 @@ class ModulePersistence extends ObservableV2<{ removed: () => void }> { if (editedRoot instanceof Ast.BodyBlock) Ast.repair(editedRoot, edit) syncModule.applyEdit(edit) } else { - const { root, spans } = Ast.parseModuleWithSpans(code, syncModule) - syncModule.syncRoot(root) - parsedSpans = spans + const metadataSnapshot = metadata.ide.snapshot + const snapshotCode = + metadataSnapshot && ModulePersistence.decodeCodeSnapshot(metadataSnapshot) + if (metadataSnapshot && idMapJson && snapshotCode && snapshotCode !== code) { + // When the received code does not match the saved code snapshot, it means that + // the code was externally edited. In this case we try to fix the spans by running + // the `syncToCode` on the saved code snapshot. + const { root, spans } = Ast.parseModuleWithSpans(snapshotCode, syncModule) + syncModule.syncRoot(root) + parsedIdMap = deserializeIdMap(idMapJson) + + const edit = syncModule.edit() + Ast.setExternalIds(edit, spans, parsedIdMap) + edit.getVersion(root).syncToCode(code) + syncModule.applyEdit(edit) + } else { + const { root, spans } = Ast.parseModuleWithSpans(code, syncModule) + syncModule.syncRoot(root) + parsedSpans = spans + } } } const astRoot = syncModule.root() if (!astRoot) return if ((code !== this.syncedCode || idMapJson !== this.syncedIdMap) && idMapJson) { const spans = parsedSpans ?? Ast.print(astRoot).info - if (idMapJson !== this.syncedIdMap) { + if (idMapJson !== this.syncedIdMap && parsedIdMap === undefined) { const idMap = deserializeIdMap(idMapJson) const idsAssigned = Ast.setExternalIds(syncModule, spans, idMap) const numberOfAsts = astCount(astRoot) @@ -646,7 +680,13 @@ class ModulePersistence extends ObservableV2<{ removed: () => void }> { this.syncedMeta = metadata this.syncedMetaJson = metadataJson }, 'file') - if (unsyncedIdMap) this.sendLsUpdate(contentsReceived, undefined, unsyncedIdMap, undefined) + if (unsyncedIdMap) + this.sendLsUpdate( + contentsReceived, + this.syncedCode ?? undefined, + unsyncedIdMap, + this.syncedMeta?.ide?.node, + ) } async close() { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d0435c3fe004..76ff20a7547a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -30,13 +30,13 @@ importers: version: 9.13.0 '@typescript-eslint/eslint-plugin': specifier: ^8.10.0 - version: 8.10.0(@typescript-eslint/parser@8.10.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3))(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) + version: 8.11.0(@typescript-eslint/parser@8.11.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3))(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) '@typescript-eslint/parser': specifier: ^8.10.0 - version: 8.10.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) + version: 8.11.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) '@vue/eslint-config-typescript': specifier: ^14.1.1 - version: 14.1.1(@typescript-eslint/parser@8.10.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3))(eslint-plugin-vue@9.29.1(eslint@9.13.0(jiti@1.21.6)))(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) + version: 14.1.3(@typescript-eslint/parser@8.11.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3))(eslint-plugin-vue@9.29.1(eslint@9.13.0(jiti@1.21.6)))(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) eslint: specifier: ^9.13.0 version: 9.13.0(jiti@1.21.6) @@ -493,7 +493,7 @@ importers: version: 4.0.0(prettier@3.3.2)(typescript@5.5.3)(vue-tsc@2.0.24(typescript@5.5.3)) prettier-plugin-tailwindcss: specifier: ^0.5.11 - version: 0.5.14(@ianvs/prettier-plugin-sort-imports@4.3.0(prettier@3.3.2))(prettier-plugin-organize-imports@4.0.0(prettier@3.3.2)(typescript@5.5.3)(vue-tsc@2.0.24(typescript@5.5.3)))(prettier@3.3.2) + version: 0.5.14(@ianvs/prettier-plugin-sort-imports@4.3.0(@vue/compiler-sfc@3.5.2)(prettier@3.3.2))(prettier-plugin-organize-imports@4.0.0(prettier@3.3.2)(typescript@5.5.3)(vue-tsc@2.0.24(typescript@5.5.3)))(prettier@3.3.2) shuffle-seed: specifier: ^1.1.6 version: 1.1.6 @@ -614,7 +614,7 @@ importers: version: 31.2.0 electron-builder: specifier: ^24.13.3 - version: 24.13.3(electron-builder-squirrel-windows@24.13.3(dmg-builder@24.13.3)) + version: 24.13.3(electron-builder-squirrel-windows@24.13.3) enso-common: specifier: workspace:* version: link:../../common @@ -674,6 +674,9 @@ importers: isomorphic-ws: specifier: ^5.0.0 version: 5.0.0(ws@8.18.0) + js-base64: + specifier: ^3.7.7 + version: 3.7.7 lib0: specifier: ^0.2.85 version: 0.2.94 @@ -3097,8 +3100,8 @@ packages: '@types/yauzl@2.10.3': resolution: {integrity: sha512-oJoftv0LSuaDZE3Le4DbKX+KS9G36NzOeSap90UIK0yMA/NhKJhqlSGtNDORNRaIbQfzjXDrQa0ytJ6mNRGz/Q==} - '@typescript-eslint/eslint-plugin@8.10.0': - resolution: {integrity: sha512-phuB3hoP7FFKbRXxjl+DRlQDuJqhpOnm5MmtROXyWi3uS/Xg2ZXqiQfcG2BJHiN4QKyzdOJi3NEn/qTnjUlkmQ==} + '@typescript-eslint/eslint-plugin@8.11.0': + resolution: {integrity: sha512-KhGn2LjW1PJT2A/GfDpiyOfS4a8xHQv2myUagTM5+zsormOmBlYsnQ6pobJ8XxJmh6hnHwa2Mbe3fPrDJoDhbA==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} peerDependencies: '@typescript-eslint/parser': ^8.0.0 || ^8.0.0-alpha.0 @@ -3108,8 +3111,8 @@ packages: typescript: optional: true - '@typescript-eslint/parser@8.10.0': - resolution: {integrity: sha512-E24l90SxuJhytWJ0pTQydFT46Nk0Z+bsLKo/L8rtQSL93rQ6byd1V/QbDpHUTdLPOMsBCcYXZweADNCfOCmOAg==} + '@typescript-eslint/parser@8.11.0': + resolution: {integrity: sha512-lmt73NeHdy1Q/2ul295Qy3uninSqi6wQI18XwSpm8w0ZbQXUpjCAWP1Vlv/obudoBiIjJVjlztjQ+d/Md98Yxg==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} peerDependencies: eslint: ^8.57.0 || ^9.0.0 @@ -3118,12 +3121,12 @@ packages: typescript: optional: true - '@typescript-eslint/scope-manager@8.10.0': - resolution: {integrity: sha512-AgCaEjhfql9MDKjMUxWvH7HjLeBqMCBfIaBbzzIcBbQPZE7CPh1m6FF+L75NUMJFMLYhCywJXIDEMa3//1A0dw==} + '@typescript-eslint/scope-manager@8.11.0': + resolution: {integrity: sha512-Uholz7tWhXmA4r6epo+vaeV7yjdKy5QFCERMjs1kMVsLRKIrSdM6o21W2He9ftp5PP6aWOVpD5zvrvuHZC0bMQ==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} - '@typescript-eslint/type-utils@8.10.0': - resolution: {integrity: sha512-PCpUOpyQSpxBn230yIcK+LeCQaXuxrgCm2Zk1S+PTIRJsEfU6nJ0TtwyH8pIwPK/vJoA+7TZtzyAJSGBz+s/dg==} + '@typescript-eslint/type-utils@8.11.0': + resolution: {integrity: sha512-ItiMfJS6pQU0NIKAaybBKkuVzo6IdnAhPFZA/2Mba/uBjuPQPet/8+zh5GtLHwmuFRShZx+8lhIs7/QeDHflOg==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} peerDependencies: typescript: '*' @@ -3131,12 +3134,12 @@ packages: typescript: optional: true - '@typescript-eslint/types@8.10.0': - resolution: {integrity: sha512-k/E48uzsfJCRRbGLapdZgrX52csmWJ2rcowwPvOZ8lwPUv3xW6CcFeJAXgx4uJm+Ge4+a4tFOkdYvSpxhRhg1w==} + '@typescript-eslint/types@8.11.0': + resolution: {integrity: sha512-tn6sNMHf6EBAYMvmPUaKaVeYvhUsrE6x+bXQTxjQRp360h1giATU0WvgeEys1spbvb5R+VpNOZ+XJmjD8wOUHw==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} - '@typescript-eslint/typescript-estree@8.10.0': - resolution: {integrity: sha512-3OE0nlcOHaMvQ8Xu5gAfME3/tWVDpb/HxtpUZ1WeOAksZ/h/gwrBzCklaGzwZT97/lBbbxJ16dMA98JMEngW4w==} + '@typescript-eslint/typescript-estree@8.11.0': + resolution: {integrity: sha512-yHC3s1z1RCHoCz5t06gf7jH24rr3vns08XXhfEqzYpd6Hll3z/3g23JRi0jM8A47UFKNc3u/y5KIMx8Ynbjohg==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} peerDependencies: typescript: '*' @@ -3144,14 +3147,14 @@ packages: typescript: optional: true - '@typescript-eslint/utils@8.10.0': - resolution: {integrity: sha512-Oq4uZ7JFr9d1ZunE/QKy5egcDRXT/FrS2z/nlxzPua2VHFtmMvFNDvpq1m/hq0ra+T52aUezfcjGRIB7vNJF9w==} + '@typescript-eslint/utils@8.11.0': + resolution: {integrity: sha512-CYiX6WZcbXNJV7UNB4PLDIBtSdRmRI/nb0FMyqHPTQD1rMjA0foPLaPUV39C/MxkTd/QKSeX+Gb34PPsDVC35g==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} peerDependencies: eslint: ^8.57.0 || ^9.0.0 - '@typescript-eslint/visitor-keys@8.10.0': - resolution: {integrity: sha512-k8nekgqwr7FadWk548Lfph6V3r9OVqjzAIVskE7orMZR23cGJjAOVazsZSJW+ElyjfTM4wx/1g88Mi70DDtG9A==} + '@typescript-eslint/visitor-keys@8.11.0': + resolution: {integrity: sha512-EaewX6lxSjRJnc+99+dqzTeoDZUfyrA52d2/HRrkI830kgovWsmIiTfmr0NZorzqic7ga+1bS60lRBUgR3n/Bw==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} '@vitejs/plugin-react@4.3.1': @@ -3242,8 +3245,8 @@ packages: '@vue/devtools-shared@7.3.7': resolution: {integrity: sha512-M9EU1/bWi5GNS/+IZrAhwGOVZmUTN4MH22Hvh35nUZZg9AZP2R2OhfCb+MG4EtAsrUEYlu3R43/SIj3G7EZYtQ==} - '@vue/eslint-config-typescript@14.1.1': - resolution: {integrity: sha512-zw6q8pXUuFHfdZsdKYr344giBRhnq6WmWSO2abFHajxR8wDX2p2hkj0/Mf6W2phTkerU4b8WF2jgq2Z/c+PdMA==} + '@vue/eslint-config-typescript@14.1.3': + resolution: {integrity: sha512-L4NUJQz/0We2QYtrNwRAGRy4KfpOagl5V3MpZZ+rQ51a+bKjlKYYrugi7lp7PIX8LolRgu06ZwDoswnSGWnAmA==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} peerDependencies: eslint: ^9.10.0 @@ -3557,6 +3560,7 @@ packages: boolean@3.2.0: resolution: {integrity: sha512-d0II/GO9uf9lfUHH2BQsjxzRJZBdsjgsBiW4BvhWk/3qoKwQFjIDVN19PfX8F2D/r9PCMTtLWjYVCFrpeYUzsw==} + deprecated: Package no longer supported. Contact Support at https://www.npmjs.com/support for more info. bowser@2.11.0: resolution: {integrity: sha512-AlcaJBi/pqqJBIQ8U9Mcpc9i8Aqxn88Skv5d+xBX006BY5u8N3mGLHa5Lgppa7L/HfwgwLgZ6NYs+Ag6uUmJRA==} @@ -5262,6 +5266,9 @@ packages: jpeg-js@0.2.0: resolution: {integrity: sha512-Ni9PffhJtYtdD7VwxH6V2MnievekGfUefosGCHadog0/jAevRu6HPjYeMHbUemn0IPE8d4wGa8UsOGsX+iKy2g==} + js-base64@3.7.7: + resolution: {integrity: sha512-7rCnleh0z2CkXhH67J8K1Ytz0b2Y+yxTPL+/KOJoa20hfnVQ/3/T6W/KflYI4bRHRagNeXeU2bkNGI3v1oS/lw==} + js-beautify@1.15.1: resolution: {integrity: sha512-ESjNzSlt/sWE8sciZH8kBF8BPlwXPwhR6pWKAw8bw4Bwj+iZcnKW6ONWUutJ7eObuBZQpiIb8S7OYspWrKt7rA==} engines: {node: '>=14'} @@ -7092,8 +7099,8 @@ packages: resolution: {integrity: sha512-/OxDN6OtAk5KBpGb28T+HZc2M+ADtvRxXrKKbUwtsLgdoxgX13hyy7ek6bFRl5+aBs2yZzB0c4CnQfAtVypW/g==} engines: {node: '>= 0.4'} - typescript-eslint@8.10.0: - resolution: {integrity: sha512-YIu230PeN7z9zpu/EtqCIuRVHPs4iSlqW6TEvjbyDAE3MZsSl2RXBo+5ag+lbABCG8sFM1WVKEXhlQ8Ml8A3Fw==} + typescript-eslint@8.11.0: + resolution: {integrity: sha512-cBRGnW3FSlxaYwU8KfAewxFK5uzeOAp0l2KebIlPDOT5olVi65KDG/yjBooPBG0kGW/HLkoz1c/iuBFehcS3IA==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} peerDependencies: typescript: '*' @@ -8782,7 +8789,7 @@ snapshots: '@humanwhocodes/retry@0.3.1': {} - '@ianvs/prettier-plugin-sort-imports@4.3.0(prettier@3.3.2)': + '@ianvs/prettier-plugin-sort-imports@4.3.0(@vue/compiler-sfc@3.5.2)(prettier@3.3.2)': dependencies: '@babel/core': 7.25.2 '@babel/generator': 7.25.6 @@ -8791,6 +8798,8 @@ snapshots: '@babel/types': 7.25.6 prettier: 3.3.2 semver: 7.6.3 + optionalDependencies: + '@vue/compiler-sfc': 3.5.2 transitivePeerDependencies: - supports-color optional: true @@ -10522,14 +10531,14 @@ snapshots: '@types/node': 20.11.21 optional: true - '@typescript-eslint/eslint-plugin@8.10.0(@typescript-eslint/parser@8.10.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3))(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3)': + '@typescript-eslint/eslint-plugin@8.11.0(@typescript-eslint/parser@8.11.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3))(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3)': dependencies: '@eslint-community/regexpp': 4.11.0 - '@typescript-eslint/parser': 8.10.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) - '@typescript-eslint/scope-manager': 8.10.0 - '@typescript-eslint/type-utils': 8.10.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) - '@typescript-eslint/utils': 8.10.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) - '@typescript-eslint/visitor-keys': 8.10.0 + '@typescript-eslint/parser': 8.11.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) + '@typescript-eslint/scope-manager': 8.11.0 + '@typescript-eslint/type-utils': 8.11.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) + '@typescript-eslint/utils': 8.11.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) + '@typescript-eslint/visitor-keys': 8.11.0 eslint: 9.13.0(jiti@1.21.6) graphemer: 1.4.0 ignore: 5.3.1 @@ -10540,12 +10549,12 @@ snapshots: transitivePeerDependencies: - supports-color - '@typescript-eslint/parser@8.10.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3)': + '@typescript-eslint/parser@8.11.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3)': dependencies: - '@typescript-eslint/scope-manager': 8.10.0 - '@typescript-eslint/types': 8.10.0 - '@typescript-eslint/typescript-estree': 8.10.0(typescript@5.5.3) - '@typescript-eslint/visitor-keys': 8.10.0 + '@typescript-eslint/scope-manager': 8.11.0 + '@typescript-eslint/types': 8.11.0 + '@typescript-eslint/typescript-estree': 8.11.0(typescript@5.5.3) + '@typescript-eslint/visitor-keys': 8.11.0 debug: 4.3.7 eslint: 9.13.0(jiti@1.21.6) optionalDependencies: @@ -10553,15 +10562,15 @@ snapshots: transitivePeerDependencies: - supports-color - '@typescript-eslint/scope-manager@8.10.0': + '@typescript-eslint/scope-manager@8.11.0': dependencies: - '@typescript-eslint/types': 8.10.0 - '@typescript-eslint/visitor-keys': 8.10.0 + '@typescript-eslint/types': 8.11.0 + '@typescript-eslint/visitor-keys': 8.11.0 - '@typescript-eslint/type-utils@8.10.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3)': + '@typescript-eslint/type-utils@8.11.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3)': dependencies: - '@typescript-eslint/typescript-estree': 8.10.0(typescript@5.5.3) - '@typescript-eslint/utils': 8.10.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) + '@typescript-eslint/typescript-estree': 8.11.0(typescript@5.5.3) + '@typescript-eslint/utils': 8.11.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) debug: 4.3.7 ts-api-utils: 1.3.0(typescript@5.5.3) optionalDependencies: @@ -10570,12 +10579,12 @@ snapshots: - eslint - supports-color - '@typescript-eslint/types@8.10.0': {} + '@typescript-eslint/types@8.11.0': {} - '@typescript-eslint/typescript-estree@8.10.0(typescript@5.5.3)': + '@typescript-eslint/typescript-estree@8.11.0(typescript@5.5.3)': dependencies: - '@typescript-eslint/types': 8.10.0 - '@typescript-eslint/visitor-keys': 8.10.0 + '@typescript-eslint/types': 8.11.0 + '@typescript-eslint/visitor-keys': 8.11.0 debug: 4.3.7 fast-glob: 3.3.2 is-glob: 4.0.3 @@ -10587,20 +10596,20 @@ snapshots: transitivePeerDependencies: - supports-color - '@typescript-eslint/utils@8.10.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3)': + '@typescript-eslint/utils@8.11.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3)': dependencies: '@eslint-community/eslint-utils': 4.4.0(eslint@9.13.0(jiti@1.21.6)) - '@typescript-eslint/scope-manager': 8.10.0 - '@typescript-eslint/types': 8.10.0 - '@typescript-eslint/typescript-estree': 8.10.0(typescript@5.5.3) + '@typescript-eslint/scope-manager': 8.11.0 + '@typescript-eslint/types': 8.11.0 + '@typescript-eslint/typescript-estree': 8.11.0(typescript@5.5.3) eslint: 9.13.0(jiti@1.21.6) transitivePeerDependencies: - supports-color - typescript - '@typescript-eslint/visitor-keys@8.10.0': + '@typescript-eslint/visitor-keys@8.11.0': dependencies: - '@typescript-eslint/types': 8.10.0 + '@typescript-eslint/types': 8.11.0 eslint-visitor-keys: 3.4.3 '@vitejs/plugin-react@4.3.1(vite@5.3.5(@types/node@20.11.21)(lightningcss@1.25.1))': @@ -10768,13 +10777,13 @@ snapshots: dependencies: rfdc: 1.4.1 - '@vue/eslint-config-typescript@14.1.1(@typescript-eslint/parser@8.10.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3))(eslint-plugin-vue@9.29.1(eslint@9.13.0(jiti@1.21.6)))(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3)': + '@vue/eslint-config-typescript@14.1.3(@typescript-eslint/parser@8.11.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3))(eslint-plugin-vue@9.29.1(eslint@9.13.0(jiti@1.21.6)))(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3)': dependencies: - '@typescript-eslint/eslint-plugin': 8.10.0(@typescript-eslint/parser@8.10.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3))(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) + '@typescript-eslint/eslint-plugin': 8.11.0(@typescript-eslint/parser@8.11.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3))(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) eslint: 9.13.0(jiti@1.21.6) eslint-plugin-vue: 9.29.1(eslint@9.13.0(jiti@1.21.6)) fast-glob: 3.3.2 - typescript-eslint: 8.10.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) + typescript-eslint: 8.11.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) vue-eslint-parser: 9.4.3(eslint@9.13.0(jiti@1.21.6)) optionalDependencies: typescript: 5.5.3 @@ -10966,7 +10975,7 @@ snapshots: app-builder-bin@4.0.0: {} - app-builder-lib@24.13.3(dmg-builder@24.13.3(electron-builder-squirrel-windows@24.13.3))(electron-builder-squirrel-windows@24.13.3(dmg-builder@24.13.3)): + app-builder-lib@24.13.3(dmg-builder@24.13.3)(electron-builder-squirrel-windows@24.13.3): dependencies: '@develar/schema-utils': 2.6.5 '@electron/notarize': 2.2.1 @@ -11884,7 +11893,7 @@ snapshots: dmg-builder@24.13.3(electron-builder-squirrel-windows@24.13.3): dependencies: - app-builder-lib: 24.13.3(dmg-builder@24.13.3(electron-builder-squirrel-windows@24.13.3))(electron-builder-squirrel-windows@24.13.3(dmg-builder@24.13.3)) + app-builder-lib: 24.13.3(dmg-builder@24.13.3)(electron-builder-squirrel-windows@24.13.3) builder-util: 24.13.1 builder-util-runtime: 9.2.4 fs-extra: 10.1.0 @@ -11965,7 +11974,7 @@ snapshots: electron-builder-squirrel-windows@24.13.3(dmg-builder@24.13.3): dependencies: - app-builder-lib: 24.13.3(dmg-builder@24.13.3(electron-builder-squirrel-windows@24.13.3))(electron-builder-squirrel-windows@24.13.3(dmg-builder@24.13.3)) + app-builder-lib: 24.13.3(dmg-builder@24.13.3)(electron-builder-squirrel-windows@24.13.3) archiver: 5.3.2 builder-util: 24.13.1 fs-extra: 10.1.0 @@ -11973,9 +11982,9 @@ snapshots: - dmg-builder - supports-color - electron-builder@24.13.3(electron-builder-squirrel-windows@24.13.3(dmg-builder@24.13.3)): + electron-builder@24.13.3(electron-builder-squirrel-windows@24.13.3): dependencies: - app-builder-lib: 24.13.3(dmg-builder@24.13.3(electron-builder-squirrel-windows@24.13.3))(electron-builder-squirrel-windows@24.13.3(dmg-builder@24.13.3)) + app-builder-lib: 24.13.3(dmg-builder@24.13.3)(electron-builder-squirrel-windows@24.13.3) builder-util: 24.13.1 builder-util-runtime: 9.2.4 chalk: 4.1.2 @@ -12415,7 +12424,7 @@ snapshots: estree-walker@3.0.3: dependencies: - '@types/estree': 1.0.5 + '@types/estree': 1.0.6 esutils@2.0.3: {} @@ -13253,6 +13262,8 @@ snapshots: jpeg-js@0.2.0: {} + js-base64@3.7.7: {} + js-beautify@1.15.1: dependencies: config-chain: 1.1.13 @@ -14167,11 +14178,11 @@ snapshots: optionalDependencies: vue-tsc: 2.0.24(typescript@5.5.3) - prettier-plugin-tailwindcss@0.5.14(@ianvs/prettier-plugin-sort-imports@4.3.0(prettier@3.3.2))(prettier-plugin-organize-imports@4.0.0(prettier@3.3.2)(typescript@5.5.3)(vue-tsc@2.0.24(typescript@5.5.3)))(prettier@3.3.2): + prettier-plugin-tailwindcss@0.5.14(@ianvs/prettier-plugin-sort-imports@4.3.0(@vue/compiler-sfc@3.5.2)(prettier@3.3.2))(prettier-plugin-organize-imports@4.0.0(prettier@3.3.2)(typescript@5.5.3)(vue-tsc@2.0.24(typescript@5.5.3)))(prettier@3.3.2): dependencies: prettier: 3.3.2 optionalDependencies: - '@ianvs/prettier-plugin-sort-imports': 4.3.0(prettier@3.3.2) + '@ianvs/prettier-plugin-sort-imports': 4.3.0(@vue/compiler-sfc@3.5.2)(prettier@3.3.2) prettier-plugin-organize-imports: 4.0.0(prettier@3.3.2)(typescript@5.5.3)(vue-tsc@2.0.24(typescript@5.5.3)) prettier@3.3.2: {} @@ -15192,11 +15203,11 @@ snapshots: is-typed-array: 1.1.13 possible-typed-array-names: 1.0.0 - typescript-eslint@8.10.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3): + typescript-eslint@8.11.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3): dependencies: - '@typescript-eslint/eslint-plugin': 8.10.0(@typescript-eslint/parser@8.10.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3))(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) - '@typescript-eslint/parser': 8.10.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) - '@typescript-eslint/utils': 8.10.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) + '@typescript-eslint/eslint-plugin': 8.11.0(@typescript-eslint/parser@8.11.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3))(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) + '@typescript-eslint/parser': 8.11.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) + '@typescript-eslint/utils': 8.11.0(eslint@9.13.0(jiti@1.21.6))(typescript@5.5.3) optionalDependencies: typescript: 5.5.3 transitivePeerDependencies: From fae18c0a0441fb1f641967694a7c23b5cb5ad5ed Mon Sep 17 00:00:00 2001 From: Sergei Garin Date: Mon, 28 Oct 2024 15:58:41 +0300 Subject: [PATCH 07/13] Add eslint-react-compiler (#11404) * Add React compiler eslint rules and fix issues across reusable components * Fix compiler errors * Remove fail-on-warnings for eslint * Set max-warnings to 41 to match the amount of warnings introduced by react-compiler * Add comment for lint task --- app/gui/package.json | 3 +- .../components/AriaComponents/Alert/Alert.tsx | 17 ++++-- .../AriaComponents/Button/Button.tsx | 3 +- .../AriaComponents/Checkbox/Checkbox.tsx | 15 ++--- .../AriaComponents/Checkbox/CheckboxGroup.tsx | 5 +- .../AriaComponents/Dialog/Popover.tsx | 2 +- .../Form/components/FormProvider.tsx | 2 + .../AriaComponents/Form/components/useForm.ts | 12 ++-- .../Inputs/Dropdown/Dropdown.tsx | 4 +- .../AriaComponents/Inputs/Input/Input.tsx | 4 +- .../Inputs/OTPInput/OTPInput.tsx | 4 +- .../components/AriaComponents/Radio/Radio.tsx | 4 +- .../AriaComponents/Radio/RadioGroup.tsx | 6 +- .../AriaComponents/Switch/Switch.tsx | 4 +- .../components/AriaComponents/Text/Text.tsx | 5 +- .../dashboard/components/Portal/Portal.tsx | 4 +- .../dashboard/components/Portal/usePortal.ts | 11 ++-- .../src/dashboard/hooks/eventCallbackHooks.ts | 10 +++- app/gui/src/dashboard/hooks/mountHooks.ts | 7 ++- app/gui/src/dashboard/hooks/syncRefHooks.ts | 14 +++-- .../src/dashboard/hooks/useLazyMemoHooks.ts | 1 - .../layouts/AssetDiffView/AssetDiffView.tsx | 7 +-- eslint.config.mjs | 11 +++- package.json | 1 + pnpm-lock.yaml | 58 +++++++++++++++++++ 25 files changed, 156 insertions(+), 58 deletions(-) diff --git a/app/gui/package.json b/app/gui/package.json index 29543619ca7e..38d737a94b8b 100644 --- a/app/gui/package.json +++ b/app/gui/package.json @@ -21,7 +21,8 @@ "build": "vite build", "build-cloud": "cross-env CLOUD_BUILD=true corepack pnpm run build", "preview": "vite preview", - "lint": "eslint . --max-warnings=0", + "//": "max-warnings set to 41 to match the amount of warnings introduced by the new react compiler. Eventual goal is to remove all the warnings.", + "lint": "eslint . --max-warnings=41", "format": "prettier --version && prettier --write src/ && eslint . --fix", "dev:vite": "vite", "test": "corepack pnpm run /^^^^test:.*/", diff --git a/app/gui/src/dashboard/components/AriaComponents/Alert/Alert.tsx b/app/gui/src/dashboard/components/AriaComponents/Alert/Alert.tsx index 9f1f4b1e37a7..38cb40c15adf 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Alert/Alert.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Alert/Alert.tsx @@ -79,13 +79,13 @@ export const Alert = forwardRef(function Alert( fullWidth, icon, variants = ALERT_STYLES, + tabIndex: rawTabIndex, + role: rawRole, ...containerProps } = props - if (variant === 'error') { - containerProps.tabIndex = -1 - containerProps.role = 'alert' - } + const tabIndex = variant === 'error' ? -1 : rawTabIndex + const role = variant === 'error' ? 'alert' : rawRole const classes = variants({ variant, @@ -95,7 +95,13 @@ export const Alert = forwardRef(function Alert( }) return ( -
+
{icon != null && (() => { if (typeof icon === 'string') { @@ -107,6 +113,7 @@ export const Alert = forwardRef(function Alert( } return
{icon}
})()} +
{children}
) diff --git a/app/gui/src/dashboard/components/AriaComponents/Button/Button.tsx b/app/gui/src/dashboard/components/AriaComponents/Button/Button.tsx index 2831a634dc64..9c9216cabdfc 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Button/Button.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Button/Button.tsx @@ -438,9 +438,10 @@ export const Button = forwardRef(function Button( const button = ( ()(goodDefaults, ariaProps, focusChildProps, { - ref, isDisabled, // we use onPressEnd instead of onPress because for some reason react-aria doesn't trigger // onPress on EXTRA_CLICK_ZONE, but onPress{start,end} are triggered diff --git a/app/gui/src/dashboard/components/AriaComponents/Checkbox/Checkbox.tsx b/app/gui/src/dashboard/components/AriaComponents/Checkbox/Checkbox.tsx index f89983b253b1..0ca7c11234c6 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Checkbox/Checkbox.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Checkbox/Checkbox.tsx @@ -20,13 +20,7 @@ import type { } from 'react' import invariant from 'tiny-invariant' import { useStore } from 'zustand' -import type { - FieldPath, - FieldStateProps, - FormInstance, - TSchema, - UseFormRegisterReturn, -} from '../Form' +import type { FieldPath, FieldStateProps, TSchema, UseFormRegisterReturn } from '../Form' import { Form } from '../Form' import { Text } from '../Text' import type { TestIdProps } from '../types' @@ -138,8 +132,7 @@ export const Checkbox = forwardRef(function Checkbox< const { store, removeSelected, addSelected } = useCheckboxContext() - // eslint-disable-next-line react-hooks/rules-of-hooks, no-restricted-syntax - const formInstance = (form ?? Form.useFormContext()) as unknown as FormInstance + const formInstance = Form.useFormContext(form) const { isSelected, field, onChange, name } = useStore(store, (state) => { const { insideGroup } = state @@ -199,7 +192,9 @@ export const Checkbox = forwardRef(function Checkbox< return ( { + mergeRefs(ref, field.ref)(el) + }} {...props} inputRef={useMergedRef(checkboxRef, (input) => { // Hack to remove the `data-testid` attribute from the input element diff --git a/app/gui/src/dashboard/components/AriaComponents/Checkbox/CheckboxGroup.tsx b/app/gui/src/dashboard/components/AriaComponents/Checkbox/CheckboxGroup.tsx index 82bb04401aab..dd5ff4047090 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Checkbox/CheckboxGroup.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Checkbox/CheckboxGroup.tsx @@ -11,7 +11,7 @@ import { forwardRef } from '#/utilities/react' import type { VariantProps } from '#/utilities/tailwindVariants' import { tv } from '#/utilities/tailwindVariants' import type { CSSProperties, ForwardedRef, ReactElement } from 'react' -import type { FieldVariantProps, FormInstance } from '../Form' +import type { FieldVariantProps } from '../Form' import { Form, type FieldPath, type FieldProps, type FieldStateProps, type TSchema } from '../Form' import type { TestIdProps } from '../types' import { CheckboxGroupProvider } from './CheckboxContext' @@ -58,8 +58,7 @@ export const CheckboxGroup = forwardRef( ...checkboxGroupProps } = props - // eslint-disable-next-line react-hooks/rules-of-hooks,no-restricted-syntax - const formInstance = (form ?? Form.useFormContext()) as FormInstance + const formInstance = Form.useFormContext(form) const styles = variants({ fullWidth, className }) const testId = props['data-testid'] ?? props.testId diff --git a/app/gui/src/dashboard/components/AriaComponents/Dialog/Popover.tsx b/app/gui/src/dashboard/components/AriaComponents/Dialog/Popover.tsx index 0b6c67c9fa2b..884efe497b47 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Dialog/Popover.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Dialog/Popover.tsx @@ -86,7 +86,7 @@ export function Popover(props: PopoverProps) { utlities.useInteractOutside({ ref: dialogRef, id: dialogId, - onInteractOutside: closeRef.current, + onInteractOutside: () => closeRef.current?.(), }) return ( diff --git a/app/gui/src/dashboard/components/AriaComponents/Form/components/FormProvider.tsx b/app/gui/src/dashboard/components/AriaComponents/Form/components/FormProvider.tsx index af276bb20f98..6fe3e215464a 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Form/components/FormProvider.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Form/components/FormProvider.tsx @@ -39,6 +39,7 @@ export function useFormContext( if (form != null && 'control' in form) { return form } else { + // eslint-disable-next-line react-compiler/react-compiler // eslint-disable-next-line react-hooks/rules-of-hooks const ctx = useContext(FormContext) @@ -56,6 +57,7 @@ export function useOptionalFormContext< Schema extends types.TSchema, >(form?: Form): Form extends undefined ? FormInstance | null : FormInstance { try { + // eslint-disable-next-line react-compiler/react-compiler return useFormContext(form) } catch { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion diff --git a/app/gui/src/dashboard/components/AriaComponents/Form/components/useForm.ts b/app/gui/src/dashboard/components/AriaComponents/Form/components/useForm.ts index 7ecd7d962de2..a40d1ce5fe44 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Form/components/useForm.ts +++ b/app/gui/src/dashboard/components/AriaComponents/Form/components/useForm.ts @@ -137,7 +137,11 @@ export function useForm( return result } - // eslint-disable-next-line react-hooks/rules-of-hooks + // We need to disable the eslint rules here, because we call hooks conditionally + // but it's safe to do so, because we don't switch between the two types of arguments + // and if we do, we throw an error. + /* eslint-disable react-compiler/react-compiler */ + /* eslint-disable react-hooks/rules-of-hooks */ const formMutation = useMutation({ // We use template literals to make the mutation key more readable in the devtools // This mutation exists only for debug purposes - React Query dev tools record the mutation, @@ -184,10 +188,8 @@ export function useForm( // eslint-disable-next-line @typescript-eslint/no-explicit-any,no-restricted-syntax,@typescript-eslint/no-unsafe-argument const formOnSubmit = formInstance.handleSubmit(formMutation.mutateAsync as any) - // eslint-disable-next-line react-hooks/rules-of-hooks const { isOffline } = useOffline() - // eslint-disable-next-line react-hooks/rules-of-hooks useOfflineChange( (offline) => { if (offline) { @@ -199,7 +201,6 @@ export function useForm( { isDisabled: canSubmitOffline }, ) - // eslint-disable-next-line react-hooks/rules-of-hooks const submit = useEventCallback( (event: React.FormEvent | null | undefined) => { event?.preventDefault() @@ -218,7 +219,6 @@ export function useForm( }, ) - // eslint-disable-next-line react-hooks/rules-of-hooks const setFormError = useEventCallback((error: string) => { formInstance.setError('root.submit', { message: error }) }) @@ -236,6 +236,8 @@ export function useForm( return form } + /* eslint-enable react-compiler/react-compiler */ + /* eslint-enable react-hooks/rules-of-hooks */ } /** Get the type of arguments passed to the useForm hook */ diff --git a/app/gui/src/dashboard/components/AriaComponents/Inputs/Dropdown/Dropdown.tsx b/app/gui/src/dashboard/components/AriaComponents/Inputs/Dropdown/Dropdown.tsx index ed0fb714c650..f82675dfe2ee 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Inputs/Dropdown/Dropdown.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Inputs/Dropdown/Dropdown.tsx @@ -169,7 +169,9 @@ export const Dropdown = forwardRef(function Dropdown( return (
{ + mergeRefs(ref, rootRef)(el) + }} onMouseDown={() => { isSelfMouseDownRef.current = true // `isFocused` cannot be used as `isFocusWithin` is set to `false` immediately before diff --git a/app/gui/src/dashboard/components/AriaComponents/Inputs/Input/Input.tsx b/app/gui/src/dashboard/components/AriaComponents/Inputs/Input/Input.tsx index 3fe98f700dd1..51fa95a79a2f 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Inputs/Input/Input.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Inputs/Input/Input.tsx @@ -139,7 +139,9 @@ export const Input = forwardRef(function Input< { className: classes.textArea(), type, name }, omit(fieldProps, 'isInvalid', 'isRequired', 'isDisabled', 'invalid'), )} - ref={mergeRefs(inputRef, privateInputRef, fieldProps.ref)} + ref={(el) => { + mergeRefs(inputRef, privateInputRef, fieldProps.ref)(el) + }} />
diff --git a/app/gui/src/dashboard/components/AriaComponents/Inputs/OTPInput/OTPInput.tsx b/app/gui/src/dashboard/components/AriaComponents/Inputs/OTPInput/OTPInput.tsx index dc9f40b75949..8820c381f52f 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Inputs/OTPInput/OTPInput.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Inputs/OTPInput/OTPInput.tsx @@ -139,7 +139,9 @@ export const OTPInput = forwardRef(function OTPInput< }, }, )} - ref={mergeRefs(fieldProps.ref, inputRef, innerOtpInputRef)} + ref={(el) => { + mergeRefs(fieldProps.ref, inputRef, innerOtpInputRef)(el) + }} render={({ slots }) => { const sections = (() => { const items = [] diff --git a/app/gui/src/dashboard/components/AriaComponents/Radio/Radio.tsx b/app/gui/src/dashboard/components/AriaComponents/Radio/Radio.tsx index 08081b6ebfd7..17d5fd28876b 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Radio/Radio.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Radio/Radio.tsx @@ -126,7 +126,9 @@ export const Radio = forwardRef(function Radio( return (