Skip to content

Conversation

@Seth0x41
Copy link
Contributor

What type of PR is this? (check all applicable)

  • ✨ Feature
  • πŸ› Bug Fix
  • πŸ“ Documentation Update
  • 🎨 Style
  • ♻️ Code Refactor
  • πŸ”₯ Performance Improvements
  • βœ… Test
  • πŸ€– Build
  • πŸ” CI
  • πŸ“¦ Chore (Release)
  • ⏩ Revert
  • 🌐 Internationalization / Translation

Description

Notes:

  • The current code doesn't support ZLS yet, but we're working on adding support for it together. Right now, the code uses SharedArrayBuffer if the browser supports it, and falls back to ArrayBuffer if it doesn't.
    This means ZLS can't be used yet, but the server can still run normally.

  • The second point is that I used a simple caching mechanism for lazy loading, to reduce lookup time to O(1).
    The source code is converted to a 32-bit integer and used as a cache key.
    I used 31 as a prime number because it provides good hash distribution, helps avoid collisions, and is fast .. Java uses it in hashCode.
    0x7fffffff is a 31-bit positive integer that keeps hash values positive and avoids overflow issues.

  • Third point, I haven’t added a formatter yet. Do you have any suggestions for one?

Looking forward to hearing your feedback. :)

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings

Added tests?

  • πŸ‘ yes
  • πŸ™… no, because they aren't needed
  • πŸ™‹ no, because I need help

Added to documentations?

  • πŸ““ docs (./docs)
  • πŸ“• storybook (./storybook)
  • πŸ“œ README.md
  • πŸ™… no documentation needed

[optional] Are there any post-deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

@netlify
Copy link

netlify bot commented Jun 16, 2025

βœ… Deploy Preview for livecodes ready!

Name Link
πŸ”¨ Latest commit f7c306e
πŸ” Latest deploy log https://app.netlify.com/projects/livecodes/deploys/686e4d13ea84750008b47591
😎 Deploy Preview https://deploy-preview-853--livecodes.netlify.app
πŸ“± Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@hatemhosny
Copy link
Collaborator

Great work (as usual!), @Seth0x41
Sorry for the late reply. I was quite busy.

Re: SharedArrayBuffer
Do we only need it for ZLS?
I see the compiler works without it. SharedArrayBuffer is mainly needed to share binary data between the main thread and web workers, which we are not currently using. I assume this will be needed for ZLS.
SharedArrayBuffer now works in LiveCodes (only on chrome) using chrome origin trial.
Sorry, the token header expired. I updated that and it should now be working (also for local development)
demo: https://livecodes.io/?x=id/2wbq233fzus
Origin trial allows using SharedArrayBuffer without needing crossOriginIsolated.
You probably need to change the detection to:

const isSharedArrayBufferSupported = typeof SharedArrayBuffer !== 'undefined';

When I enable it, I get an error which seems to be related to this code

Re: cache
I like that very much. Well done.

Re: formatting
ZLS should provide that.

I have some relatively minor comments. I will add these on code.

Please run npm run fix to fix failing CI tests.

Thank you :)

export const cppWasmBaseUrl = /* @__PURE__ */ getUrl('@chriskoch/[email protected]/');

export const csharpWasmBaseUrl = /* @__PURE__ */ getUrl('@seth0x41/[email protected]/');
export const zigWasmBaseUrl = /* @__PURE__ */ getUrl('@seth0x41/[email protected]/');
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file is sorted alphabetically to easily find the variables.
Please move this to the end of the file.

Comment on lines +4 to +6
const JS_UNTAR_URL = 'https://unpkg.com/[email protected]/build/dist/untar.js';
const WASI_SHIM_URL = 'https://unpkg.com/@bjorn3/[email protected]/dist/index.js';

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please move these to vendors.ts.
Also use the getUrl function to allow dynamically using the selected CDN.
e.g.
const jsUntarUrl = /* @__PURE__ */ getUrl('[email protected]/build/dist/untar.js');

livecodes.zig ??= {};

// check if SharedArrayBuffer supported by the browser else it will use ArrayBuffer
const isSharedArrayBufferSupported = typeof SharedArrayBuffer !== 'undefined' && window.crossOriginIsolated;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can remove checking for corssOriginIsolated.

Suggested change
const isSharedArrayBufferSupported = typeof SharedArrayBuffer !== 'undefined' && window.crossOriginIsolated;
const isSharedArrayBufferSupported = typeof SharedArrayBuffer !== 'undefined';

Comment on lines +124 to +133
const loadScript = (url: string): Promise<void> =>
new Promise<void>((resolve, reject) => {
const script = document.createElement('script');
script.src = url;
script.async = true;
script.onload = () => resolve();
script.onerror = () => reject(new Error(`Failed to load ${url}`));
document.head.appendChild(script);
});

Copy link
Collaborator

Choose a reason for hiding this comment

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

We already have a util function for that here: https://github.com/Seth0x41/livecodes/blob/dbfc085b293b2b6594f7625ddd5e0ab5719e665d/src/livecodes/utils/utils.ts#L175

await loadScript(url, 'untar');

if (!(window as any).WASI) {
const wasiModule = await import(WASI_SHIM_URL);
['WASI', 'File', 'Directory', 'OpenFile', 'PreopenDirectory', 'ConsoleStdout'].forEach(
(key) => wasiModule[key] && ((window as any)[key] = wasiModule[key])
Copy link
Collaborator

Choose a reason for hiding this comment

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

I prefer not adding many global variables so that we do not pollute the user environment.
Please keep this in a local variable scoped inside the script.
If you really need to add global variables, please add them to window.livecodes.

Comment on lines +185 to +188
const fileData = isSharedArrayBufferSupported
? createFile(entry.buffer)
: new File(entry.buffer);

Copy link
Collaborator

Choose a reason for hiding this comment

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

createFile already checks for isSharedArrayBufferSupported and uses the same logic.

Suggested change
const fileData = isSharedArrayBufferSupported
? createFile(entry.buffer)
: new File(entry.buffer);
const fileData = createFile(entry.buffer);

extensions: ['zig'],
editor: 'script',
largeDownload: true,
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

We are not using prettier for formatting.
We should use formatter key instead and use ZLS there.

extensions: ['zig'],
editor: 'script',
largeDownload: true,
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is used as a label for the language in the UI for compiler code viewer.
Let's keep it zig.

Suggested change
};
compiledCodeLanguage: 'zig',

extensions: ['zig'],
editor: 'script',
largeDownload: true,
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

ZLS will provide highlighting for codemirror editor only.
We can tell the code editor to use syntax highlighting for Rust which has a relatively similar syntax.
We can then override that in codemirror.
We have to add rust to the Language type.

Suggested change
};
editor: 'script',
editorLanguage: 'rust',

prologStarter,
blocklyStarter,
diagramsStarter,
zigWasmStarter,
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's put it after C++
I try to keep related languages together.

@hatemhosny
Copy link
Collaborator

I found this: https://github.com/PlasmoHQ/prettier-plugin-zig
I did not try it.

content: `
<div class="container">
<h1>Hello, <span id="name">World</span>!</h1>
<img class="logo" alt="logo" src="http://127.0.0.1:8080/livecodes/assets/templates/zig.svg" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to use baseUrl instead of absolute path for images in templates

Suggested change
<img class="logo" alt="logo" src="http://127.0.0.1:8080/livecodes/assets/templates/zig.svg" />
<img class="logo" alt="logo" src="{{ __livecodes_baseUrl__ }}assets/templates/zig.svg" />

Comment on lines +4 to +5
name: 'zig-wasm',
title: 'Zig (Wasm) Starter',
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can just call the starter template zig.

Suggested change
name: 'zig-wasm',
title: 'Zig (Wasm) Starter',
name: 'zig',
title: 'Zig Starter',

Copy link
Collaborator

Choose a reason for hiding this comment

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

We would then need to change the TS type and other mentions of the template name (e.g. in docs, language-info and command menu) to use the new template name.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 9, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
4 Security Hotspots

See analysis details on SonarQube Cloud

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.

2 participants