Skip to content

fix: improve support for isolatedModules flag #413

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pierluca wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

fix: improve support for isolatedModules flag #413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
22cdf13
fix: jsdoc type
Tyratox Jan 9, 2026
a507c39
chore: simplify imports
Tyratox Jan 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/scratch-gui/src/index-standalone.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import {EditorState} from './lib/editor-state';
import {ReactComponentLike} from 'prop-types';
import {compose} from 'redux';

export {EditorState, EditorStateParams} from './lib/editor-state';
export {AccountMenuOptions} from './lib/account-menu-options';
export {EditorState, type EditorStateParams} from './lib/editor-state';
export {type AccountMenuOptions} from './lib/account-menu-options';

export {setAppElement} from 'react-modal';

Expand Down
2 changes: 1 addition & 1 deletion packages/scratch-gui/src/lib/save-project-to-server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import xhr from 'xhr';
/**
* Save a project JSON to the project server.
* This should eventually live in scratch-www.
* @param {string} projectHost the hostname of the project service.
* @param {string|undefined} projectHost the hostname of the project service.
Copy link
Contributor

@georgyangelov georgyangelov Jan 15, 2026

Choose a reason for hiding this comment

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

Is it truly possible to have this as undefined? Wouldn't that result in `${projectHost}/${qs}` being undefined/... below?

Copy link

@Tyratox Tyratox Jan 17, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

It probably should never be, I agree but the typing in

scratch-editor/packages/scratch-gui/src/lib/legacy-storage.ts

Lines 6 to 9 in f7e83ec

import saveProjectToServer from '../lib/save-project-to-server';
export class LegacyStorage implements GUIStorage {
private projectHost?: string;

with callsite

scratch-editor/packages/scratch-gui/src/lib/legacy-storage.ts

Line 76 in f7e83ec

return saveProjectToServer(this.projectHost, projectId, vmState, params);

is such that it may pass undefined. I agree that it would be good to also fix this there but this small change is effectively just making sure that the typing in the different files is compatible. Depending on the typescript compiler, the build will otherwise file since jsdoc is used to infer type information in js files: https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html

Copy link
Contributor

@georgyangelov georgyangelov Jan 19, 2026

Choose a reason for hiding this comment

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

@Tyratox the line that you link in legacy-storage.ts performs a check and only calls saveProjectToServer if the projectHost is defined. Or that's the intention at least.. given that it's using this.projectHost in both places I see how some TS version can consider it a different check and not narrow the type.

Screenshot 2026-01-19 at 11 51 39

I suggest instead of allowing undefined as a parameter, change the saveProject function to:

const host = this.projectHost;

if (!host) {
    return Promise.reject(new Error('Project host not set'));
}
// Haven't inlined the code here so that we can keep Git history on the implementation, just in case
return saveProjectToServer(host, projectId, vmState, params);

That way TS will 100% know that the host variable cannot be undefined at the time where saveProjectToServer is called.

I believe that is the only call to the saveProjectToServer

* @param {number} projectId the ID of the project, null if a new project.
* @param {object} vmState the JSON project representation.
* @param {object} params the request params.
Expand Down
Loading