fix: Remove <null> from Value to fix TopLevelSpec consumed by code using strict mode. #7352
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I think removing the null is cleaner. Can you check why we have null there? |
|
Great, pushed an update that just removes the |
|
It was introduced here: b404b86 Looking at that change, I think it's safe to remove the |
|
Can you rebuild the schema |
|
Just did that, but it didn't produce any changes I could commit. Is there a way to trigger the tests again? |
Great if there are no changes! Before and after, is there any externally observable change? Can you assign a value that could not be assigned before or vice versa? To run the test in your fork, you may need to add a github token. Buf it there are no changes, then we should be good. Let me know and I will review the changes carefully. |
|
I copied this pull request to #7357 |
|
This change makes a difference (#7357 (comment)) but since the schema is not affected, this is ok. Thank you for the pull request. |
|
Thanks for merging and doing the follow up! |
|
Do you have a usual release schedule? I'm just wondering if you can estimate when this fix will make it into a release? Thanks! |
|
We do not and I want to get a few more things into the 5.1 release. I expect a release in the next two weeks, though. |
|
Sounds great, thanks! |
jasongrout
added a commit
to jasongrout/jupyterlab
that referenced
this pull request
Jul 9, 2021
Strict null checking is giving some errors when compiling the docs. For example vega/vega-lite#7352 and another one: Error: buildutils/src/local-repository.ts:113:34 - error TS2345: Argument of type '{ prev_npm: string; prev_yarn: string; pid: number | undefined; }' is not assignable to parameter of type 'JSONObject'. Property 'pid' is incompatible with index signature. Type 'number | undefined' is not assignable to type 'JSONValue'. Type 'undefined' is not assignable to type 'JSONValue'. 113 utils.writeJSONFile(info_file, data); ~~~~ Error: node_modules/vega-lite/build/src/vega.schema.d.ts:34:19 - error TS2344: Type 'null' does not satisfy the constraint 'SignalRef | ExprRef'. 34 value?: Value<null>; ~~~~
jasongrout
added a commit
to jasongrout/jupyterlab
that referenced
this pull request
Jul 9, 2021
Strict null checking is giving some errors when compiling the docs. For example vega/vega-lite#7352 and another one: Error: buildutils/src/local-repository.ts:113:34 - error TS2345: Argument of type '{ prev_npm: string; prev_yarn: string; pid: number | undefined; }' is not assignable to parameter of type 'JSONObject'. Property 'pid' is incompatible with index signature. Type 'number | undefined' is not assignable to type 'JSONValue'. Type 'undefined' is not assignable to type 'JSONValue'. 113 utils.writeJSONFile(info_file, data); ~~~~ Error: node_modules/vega-lite/build/src/vega.schema.d.ts:34:19 - error TS2344: Type 'null' does not satisfy the constraint 'SignalRef | ExprRef'. 34 value?: Value<null>; ~~~~
jasongrout
added a commit
to jasongrout/jupyterlab
that referenced
this pull request
Jul 9, 2021
Strict null checking is giving some errors when compiling the docs. For example vega/vega-lite#7352 and another one: Error: buildutils/src/local-repository.ts:113:34 - error TS2345: Argument of type '{ prev_npm: string; prev_yarn: string; pid: number | undefined; }' is not assignable to parameter of type 'JSONObject'. Property 'pid' is incompatible with index signature. Type 'number | undefined' is not assignable to type 'JSONValue'. Type 'undefined' is not assignable to type 'JSONValue'. 113 utils.writeJSONFile(info_file, data); ~~~~ Error: node_modules/vega-lite/build/src/vega.schema.d.ts:34:19 - error TS2344: Type 'null' does not satisfy the constraint 'SignalRef | ExprRef'. 34 value?: Value<null>; ~~~~
blink1073
pushed a commit
to jupyterlab/jupyterlab
that referenced
this pull request
Jul 9, 2021
* Restrict version of linting packages When refreshing the yarn.lock files, with ^ semver ranges on linters, it upgraded the lint packages to newer versions that had different linting decisions (causing lots of formatting changes). I think upgrading lint to new linting decisions should be a very explicit decision (not just refreshing yarn, but actually updating required versions), since lots of formatting changes can be effected. * Restrict storybook version * Refresh yarn.lock This was done by deleting the lock file and installing fresh. * Fix linting error. This fixes one place where a promise was not handled. * Turn off strict null checking for typedoc. Strict null checking is giving some errors when compiling the docs. For example vega/vega-lite#7352 and another one: Error: buildutils/src/local-repository.ts:113:34 - error TS2345: Argument of type '{ prev_npm: string; prev_yarn: string; pid: number | undefined; }' is not assignable to parameter of type 'JSONObject'. Property 'pid' is incompatible with index signature. Type 'number | undefined' is not assignable to type 'JSONValue'. Type 'undefined' is not assignable to type 'JSONValue'. 113 utils.writeJSONFile(info_file, data); ~~~~ Error: node_modules/vega-lite/build/src/vega.schema.d.ts:34:19 - error TS2344: Type 'null' does not satisfy the constraint 'SignalRef | ExprRef'. 34 value?: Value<null>; ~~~~ * Update reference UI screenshots to reflect a single launcher tab not having a close “X”. In the latest 3.0 release, if a single launcher tab is open, it does not have a close X in the tab (since we must have at least one tab open). In master, the close X does appear, but does nothing when clicking on it. After #10516, the 3.0 behavior is restored: if a single launcher tab is open, it does not have an X.
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to #6912.
When you consume
TopLevelSpeclikeimport type { TopLevelSpec } from 'vega-lite';in a project withstrictNullChecksset totrue, you'll run into the following error:The problem seems to be that the generic
ESfor theValuetype doesn't includenull. Changing that type toValue<ES extends ExprRef | SignalRef | null = ExprRef | SignalRef | null>fixes it.An alternative would be to remove the
<null>from the code that referencesValuesince the type definition includesExprRef | SignalRefas a default anyway.Let me know which version would make more sense and I'll update the PR.