Skip to content

Conversation

@3coins
Copy link
Collaborator

@3coins 3coins commented May 19, 2025

Fixes #66.

Comment on lines +122 to +136
// @ts-ignore
this._trusted = !!options.trusted;
// @ts-ignore
this.contentFactory =
options.contentFactory || OutputAreaModel.defaultContentFactory;
this.list = new ObservableList<IOutputModel>();
// @ts-ignore
this.list.changed.connect(this._onListChanged, this);
if (options.values) {
// Create an array to store promises for each value
const valuePromises = options.values.map((value, originalIndex) => {
console.log("originalIndex: ", originalIndex, ", value: ", value);
// If value has a URL, fetch the data, otherwise just use the value directly
const valuePromises = options.values.map((value, index) => {
console.debug('output #${index}, value: ${value}');
// @ts-ignore
if (value.metadata?.url) {
// @ts-ignore
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are all of these @ts-ignores needed?

Copy link
Collaborator

@Zsailer Zsailer left a comment

Choose a reason for hiding this comment

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

Pulled this down.

Works like a charm! 🚀.

Left some questions inline, but shouldn't block merging.

@ellisonbg
Copy link
Collaborator

I also tested locally and it fixes all of the outputs issues we were seeing. Nicely done! @3coins can I merged or are you finishing up the work?

@ellisonbg
Copy link
Collaborator

@Zsailer an you do a new release once this is merged?

@Zsailer
Copy link
Collaborator

Zsailer commented May 19, 2025

@ellisonbg yep! Happy to release once this is merged.

@3coins
Copy link
Collaborator Author

3coins commented May 19, 2025

Just checking on those @ts-ignore statements, will merge shortly.

@3coins
Copy link
Collaborator Author

3coins commented May 19, 2025

I see multiple eslint errors on notebook.ts, there is a way to fix these to make the code ts compliant (interface augmentation), but will need additional changes that will take longer for me to validate.

/Users/pijain/projects/jupyter-rtc-core/src/notebook.ts
   12:1   error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
   21:11  error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
   25:11  error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
   43:13  error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
   53:13  error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
   55:15  error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
   61:19  error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
   65:17  error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
   70:15  error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
   81:7   error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
   84:7   error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
   87:5   error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
   96:5   error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
  103:3   error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
  105:5   error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
  110:1   error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
  122:5   error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
  124:5   error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
  128:5   error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
  134:9   error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
  136:11  error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
  158:13  error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
  161:13  error  Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment

✖ 88 problems (85 errors, 3 warnings)
  60 errors and 0 warnings potentially fixable with the `--fix` option.

Merging this PR, so folks can continue with testing, opened #71 to track these errors.

@3coins 3coins merged commit 4c3f4f0 into jupyter-ai-contrib:main May 19, 2025
2 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Only last executed cell's output is shown on notebook re-open

3 participants