fix: improve support for isolatedModules flag

[pierluca]

Resolves

As part of our development, we made use of the TypeScript isolatedModules flag and this raised an issue when compiling the project along the lines of:

Re-exporting a type when the '--isolatedModules' flag is provided requires using 'export type'.ts

Proposed Changes

This PR implements the export type changes as discussed in this Stackoverflow thread, as well as a minor JSDoc fix.

Reason for Changes

These changes are a tiny step in the direction of supporting isolatedModules in Scratch and they improve correctness.

Test Coverage

I'm not sure what would make sense here, but I'm open to suggestions

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[pierluca]

I have read the CLA Document and I hereby sign the CLA

[Tyratox]

I have read the CLA Document and I hereby sign the CLA

[KManolov3]

Looks good to me!

cc: @cwillisf @georgyangelov in case you have an opinion on this

[georgyangelov]

Export looks good, I left a question for the JSDoc type change

[georgyangelov]

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

[Tyratox]

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

[georgyangelov]

@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.

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