Forem: Uday Rana

Remix Vite migration pains

Uday Rana — Tue, 08 Apr 2025 22:15:29 +0000

I've been trying to migrate our project Starchart from Classic Remix Compiler to Remix Vite.

Upgrade Classic Remix Compiler to Remix Vite #843

uday-rana posted on Feb 20, 2025

WIP

References

Remix Vite: Migrating How to Fix CJS/ESM Bugs in Remix

View on GitHub

I started a few weeks ago and then gave up because it was too hard. Our custom server is written in TypeScript and I could not figure out how to get it to build and play nice with the Vite build. All of the official examples write the custom server in JavaScript.

Recently I had another go at it and thanks to mihaiandrei97's example setup, which I found the link to in a GitHub discussion, I did manage to get the server to successfully build and unit tests to pass, but the server would fail during end-to-end tests every now and then, even when changing completely unrelated things like the order of our npm scripts in package.json.

I also tried the Remix Express Vite plugin and couldn't get it to work either - the server just wouldn't start even though the background services did. I noticed the plugin docs said the createServer property accepts an async function as a callback but the types don't allow it. I filed an issue on the repo to get more info.

createExpressApp.createServer does not accept async function as argument #44

uday-rana posted on Apr 08, 2025

Hello,

I want to perform some async operations before starting my server:

createServer: async (app: Application) => {
    const port = process.env.PORT || 8080;

    await services.init()
    return app.listen(port);
  },

Based on the README, createExpressApp.createServer should accept async functions as arguments

createServer?: async (app: Application) => Server

But according to the type definition it doesn't:

export type CreateServerFunction = (app: Application) => Server;

And instead when trying to pass an async function, I get the error

Type '(app: Application) => Promise<Server<typeof IncomingMessage, typeof ServerResponse>>' is not assignable to type 'CreateServerFunction'.
  Type 'Promise<Server<typeof IncomingMessage, typeof ServerResponse>>' is missing the following properties from type 'Server': listen, close, address, getConnections, and 21 more.

Are async functions not supported or is the type wrong?

View on GitHub

So with that ruled out the only solutions I could think of were to either run the server with tsx as mentioned in Remix's blog or to convert our custom server to JavaScript, like their examples.

While I wait to hear back from the others on what they think the better approach is, I'm going to switch gears and work on the other issue for this sprint, which is looking into passkey encryption for users' API keys in our other project chatcraft.org.

Encrypt users' API keys with passkeys #872

uday-rana posted on Apr 06, 2025

Earlier this week @humphd suggested encrypting users' API keys with passkeys using age-encryption, which will eventually let us store them in the backend.

View on GitHub

Planning the Last Sprint

Uday Rana — Thu, 03 Apr 2025 18:11:51 +0000

For this last sprint in OSD700, there are two things I want to work on.

Starchart

For Starchart, I want to try and finish the migration I was working on from Classic Remix Compiler to Remix Vite, so that we can include it in the release next week.

Upgrade Classic Remix Compiler to Remix Vite #843

uday-rana posted on Feb 20, 2025

WIP

References

Remix Vite: Migrating How to Fix CJS/ESM Bugs in Remix

View on GitHub

Last I worked on it, I managed to get the dev server to run fine, but something's still broken in the build step, because the server gets stuck starting up forever after being built.

I'm not sure how to debug this yet but I'd really like to get it in as part of the release so I expect to spent a fair amount of time on it this week.

chatcraft.org

The other thing I'm doing for this sprint is looking into implementing age encryption for API keys in chatcraft.org, which will allow for securely storing them on the server instead of in browser storage. This should be pretty interesting because I haven't had a lot of opportunities to write new code this term, and I'll have to do some research into the implementation since I haven't worked with age encryption before.

Fixing a regression with git bisect

Uday Rana — Sun, 30 Mar 2025 00:14:45 +0000

This sprint I worked on chatcraft.org where I had the opportunity to fix a regression using git bisect for the first time.

tarasglek / chatcraft.org

Developer-oriented ChatGPT clone

ChatCraft.org

Welcome to ChatCraft.org, your open-source web companion for coding with Large Language Models (LLMs). Designed with developers in mind, ChatCraft transforms the way you interact with GPT models, making it effortless to read, write, debug, and enhance your code.

Whether you're exploring new designs or learning about the latest technologies, ChatCraft is your go-to platform. With a user interface inspired by GitHub, and editable Markdown everywhere, you'll feel right at home from the get-go.

Features

We think ChatCraft is the best platform for learning, experimenting, and getting creative with code. Here's a few of the reasons why we think you'll agree:

🛠️ You're in Control: Customize all aspects of a chat. Use your own System Prompts, edit, delete, and retry AI messages with models from competing vendors in the same chat.

🌍 Multiple AI Providers: ChatCraft supports both OpenAI and OpenRouter, giving you access to a…

View on GitHub

Issue 1

I mostly spent time looking into this issue which listed a number of bugs in the user settings modal:

openrouter mostly broken in settings #848

tarasglek posted on Mar 04, 2025

For repro

use chatcraft.org in private window.
open settings

Click on checkbox next to openrouter

it selects openai

Click on openrouter api key box

It now shows up the magic "get openrouter key" box...We should probably surface that better
Click it, login
Observe how openrouter key was not added

View on GitHub

This issue outlined three bugs:

Clicking any of the checkboxes for choosing the LLM provider in the settings modal would always result in the first provider being selected.
The "Get OpenRouter Key" button would only show when the API key input field was focused, which was bad for the option's discoverability.
The button doesn't actually work.

Bug 1: Broken LLM selection in settings

I spent two days looking into this bug. I had a hunch it was a regression because I figured surely the UI for this must have worked at some point, but there hadn't been any recent changes to the component. I tried to investigate by checking out the last commit that touched the LLM selection code, but I ran into errors installing the dependencies. pnpm said it couldn't find the dependencies, which I assume was because the versions were just that old.

After that I gave up on the regression idea (in hindsight, I should've kept digging) and instead tried looking into the current code. I had no luck trying to debug the existing logic, so I tried to re-create the component from scratch. I noticed it worked until I wrapped the <Table> element which contains the checkboxes in a <FormControl>, both of these being Chakra UI components. So I figured the bug must be in the JSX being rendered and not in the component logic, but again, there hadn't been any recent changes to the JSX.

Since we were using Chakra UI I wondered if the behaviour of the components we were using might have changed, so I tried finding a commit without the bug again. I checked out a random commit from this past September and built and ran the app, and sure enough, the bug wasn't there, which confirmed that this was a regression.

This meant I finally had an opportunity to use git bisect to find the commit that caused the bug. git bisect is a git command that uses binary search to identify which commit introduced a bug in your program. I was excited to try it out, and it led me to this commit which updated a bunch of dependencies:

Update deps for March 2025 #847

humphd posted on Mar 02, 2025

Updating deps for March, most things are easy, but I had to leave apache-arrow at 17 vs. 19 for duckdb compat, and the new TypeScript isn't compatible with eslint yet (PR hasn't landed). The rest is probably fine to leave.

Please test and make sure this works for you.

View on GitHub

The most obvious suspect was Chakra UI being bumped from 2.10.5 to 2.10.6. I tested with 2.10.5 and the bug wasn't there - then, I installed 2.10.6, and it stopped working, which confirmed my suspicions. I tried looking for release notes for this version, but there weren't any - not on GitHub, and not on their website. I went to their issues page on GitHub and searched for "2.10.5" which led me to this issue:

CheckboxGroup breaks when wrapped by FormControl: Electric Boogaloo #9747

EmilRybergAkson posted on Feb 28, 2025

Description

Seems like the same bug from #8938 has reappeared in 2.10.6. 2.10.3-2.10.5 all work.

Link to Reproduction

https://codesandbox.io/p/devbox/wonderful-dan-forked-yl6kxl

Chakra UI Version

2.10.6

Browser

Google Chrome 133

Operating System

[ ] macOS
[x] Windows
[ ] Linux

View on GitHub

One of the maintainers of Chakra UI commented on this saying that this was patched in 2.10.7, so I updated to it and the bug was fixed.

Fix LLM provider selection bug #865

uday-rana posted on Mar 28, 2025

Related issue: #848

I was able to bisect the selection issue to https://github.com/tarasglek/chatcraft.org/commit/682acced4bde6a5d1289be3df1142969fb046090 (https://github.com/tarasglek/chatcraft.org/pull/847).

The bug originated when updating @chakra-ui/react from 2.10.5 to 2.10.6. An issue was filed for this bug at https://github.com/chakra-ui/chakra-ui/issues/9747

This issue was addressed in 2.10.7. Upgrading fixes the problem.

View on GitHub

Even though the fix ultimately ended up being really simple, I'm glad I knew about git bisect because I don't think I could have found the cause of this bug without it. I probably would've kept looking for potentially breaking changes in the settings component or related components.

Bug 2: "Get OpenRouter Key" button only shows when OpenRouter API key field is focused

I'm not sure if this counts as a bug since this was an intentional decision, but I think making the button always visible is a good idea so users can find out that option exists in the first place. To do this, I just had to remove a condition that checked whether the API key field was focused before displaying it. I finished this before the selection bug and made a separate pull request since I wasn't sure at the time whether I'd be able to fix that one.

In User Settings > Models > LLM Providers, show "Get OpenRouter Key" button even when OpenRouter is not selected #864

uday-rana posted on Mar 28, 2025

Related: #848

Button still doesn't actually get the key but I figured this might be worth landing separately

View on GitHub

The third bug, the button not working, is something I didn't have time to work on this sprint. It might be worth investigating next sprint.

Issue 2

Although well overdue, I finished the issue I signed up for last sprint, which was refactoring the program to use the new useChat custom React hook.

Refactor Code To Use `useChat()` Hook #835

mulla028 posted on Feb 20, 2025

Description

Since @humphd landed #822, introduced ChatProvider & useChat() hook. We need to refactor code to stop passing chat on props in components and utilize useChat() hook wherever it's possible .

View on GitHub

I spent a while trying to fit the hook into places where it didn't make sense and in the end only found one place where using it did make sense. The hook gets a chat object by getting it's ID from the route parameter and making a database query, but most components in the code receive the chat through props from two higher-level components. Those higher-level components get chat using React Router's loader function, meaning I couldn't swap their logic out for the hook without making the code more complicated which would defeat the purpose of the refactor. Even if I did, we'd still need to drill the chat prop to their child components, because the logic for chat being undefined is only handled in the higher-level components and handling it at every level would once again just make the code messier.

Use useChat in MessageBase.tsx #860

uday-rana posted on Mar 26, 2025

Fixes #835

The useChat hook can't be used in any components that are children of ChatBase. ChatBase receives the chat prop from it's parent components LocalChat and RemoteChat. These components retrieve chat using useLoaderData(), while the hook retrieves chat from the route parameter using useParams(). From what I understand, this means the hook can't replace the usage in LocalChat and RemoteChat.

This leaves MessageBase as the only component where useChat can be used in it's current implementation.

Using the hook in LocalChat and RemoteChat will require refactoring them and their usage in router.tsx. I'll try this and see if it makes sense to do this.

View on GitHub

Extra

I also tried looking into this issue until I noticed it had already been fixed and just wasn't closed:

Add email to GitHub token info #855

humphd posted on Mar 12, 2025

When a user logs in, we need to also grab the email for the user and include in the data. The Google auth function already does this.

View on GitHub

There was a typo mentioned in one of the comments on the issue which wasn't fixed, so I went ahead and fixed that while I was working on the other stuff.

Fix "ouath" typo #861

uday-rana posted on Mar 27, 2025

View on GitHub

Progress Update

Uday Rana — Thu, 13 Mar 2025 14:54:35 +0000

I wasn't able to get as much done this sprint as I hoped. Here's what I did:

chatcraft.org

I tried doing some work in chatcraft.org but got stuck. I was trying to refactor the code to use a custom hook to manage state instead of prop drilling.

Refactor Code To Use `useChat()` Hook #835

mulla028 posted on Feb 20, 2025

Description

Since @humphd landed #822, introduced ChatProvider & useChat() hook. We need to refactor code to stop passing chat on props in components and utilize useChat() hook wherever it's possible .

View on GitHub

Unfortunately I ran into some trouble with the types. The hook can potentially return undefined, which the code doesn't handle, since when it's being passed through props that's not something that needs to be handled.

I'll need to dedicate some more time to this.

Mattermost

I did some fixes on my PR in Mattermost.

[MM-53650] Add disable emoticon rendering setting to webapp #29414

uday-rana posted on Nov 28, 2024

Summary

This pull request adds a user setting to the webapp to toggle rendering emoticons (:D) as emojis (😄).

The setting is added as a component in components/user_settings/display/render_emoticons_as_emoji/ which is imported in components/user_settings/display/user_settings_display.tsx.

I've added a renderOnOffLabel() function to user_settings_display.tsx, lifted from components/user_settings/advanced/user_settings_advanced.tsx to help render the new component.

The setting is stored as a user preference using the savePreferences() action.

I've added constants for the preference to utils/constants.tsx and webapp/channels/src/packages/mattermost-redux/src/constants/preferences.ts.

To actually use the setting, I've modified components/post_markdown to receive it's value as a prop, for which I've used getBool() and added a default value to the config. post_markdown passes this value down to Markdown on the options object, which then passes it down to utils/text_formatting.tsx, which finally passes the value to emoticons.tsx as a newly added parameter. emoticons.tsx checks whether the value is true and if it is, it transforms the emoticons into emojis.

I've updated affected tests and created unit tests for the new component. I've also updated the English translation file.

QA Test Steps

Navigate to User Settings.
Go to the Display category.
Find the section labelled "Auto-render emoticons as emoji" and click "Edit".
Toggle the setting and click "Save".
Emoticon rendering on messages sent by the current user and other users should be toggled client-side with the setting.

Ticket Link

Fixes (partially) https://github.com/mattermost/mattermost/issues/26504 Jira https://mattermost.atlassian.net/browse/MM-53650

Note the issue and ticket describe adding this feature to the mobile app as well, which this PR does not.

Screenshots

before	after

Release Note

Added a new user setting to toggle rendering emoticons (:D) as emojis (😄)

View on GitHub

The useEffect callback in my component sets focus on a link within the component, but it's supposed to be called when the component is interacted with instead of on the initial render. As it was, as soon as the component was rendered, it would focus on the link, meaning the page would scroll down to the component instead of starting at the top. The maintainers caught this bug and recommended I use a hook they wrote, which as far as I understand, works like useEffect but skips the initial render.

I switched from useEffect to their hook and re-requested a review.

Back to Work

Uday Rana — Thu, 06 Mar 2025 05:08:14 +0000

I've been spending my break away from the computer and so I haven't worked on Starchart in a minute. Here's the latest and what I want to do next.

Pull Requests

MX Record support: This was the big feature I planned to work on but didn't get to. My team member Theo took the initiative on this so I'm going to work with him to see this through.

Add ability to handle MX records #852

theoforger posted on Mar 04, 2025

Closes #787.

Description

This PR implements the feature to manage MX records. It also includes relevant unit tests and a new section in the instructions page.

References

View on GitHub

Remix Vite migration: I was in the middle of working on this the last time I worked on Starchart, but there were many, many breaking changes and it proved quite overwhelming. I found a blog detailing somebody else's experience migrating to Remix Vite, so I'm going to spend some more time and see if I can figure this out.

Upgrade Classic Remix Compiler to Remix Vite #843

uday-rana posted on Feb 20, 2025

WIP

References

Remix Vite: Migrating How to Fix CJS/ESM Bugs in Remix

View on GitHub

In the meantime, I've filed a pull request for an intermediate step - moving from remix watch to remix dev. This makes our npm scripts a lot simpler:

Upgrade `remix watch` to `remix dev` #842

uday-rana posted on Feb 20, 2025

This is an intermediate step in upgrading to Remix Vite.

Changes

Upgrade remix watch to remix dev
- We were supposed to do this when upgrading to v2
Use tsx to run server.ts and watch for changes instead of building it first with esbuild
- Remix examples use a .js file for the server so I went to the Remix Discord server and they recommended this method. However, this is slower than building the server.
- We no longer need predev, dev:build, delay, dev:remix, dev:server:delay, dev:server scripts
Remove purgeRequireCache
- According to the video walkthrough linked below this is no longer required.
Remove --require ./node_modules/dotenv/config
- According to the video walkthrough linked below, this is no longer required because Remix automatically loads .env files.
Replace ts-node with tsx in prisma.seed
- We already had ts-node to run TypeScript files, but Remix recommends tsx and there's no reason to keep both

References

Upgrading to v2: remix dev: Custom app server @remix-run/dev CLI: remix dev: With custom app server Migrating your project to v2_dev 🚚

View on GitHub

Issues filed

I filed a couple of issues - one for a new feature, and one for a known bug I learned a bit more about:

Support for creating DNS records for user.starchart.com: This is a new feature I filed an issue for. Amazon Cognito, an auth service, requires the parent domain to have an A record, in order to use the subdomain as custom domain for it's managed login. So to use project1-auth.user.starchart.com, user.starchart.com must have an A record. If we want to support this use case we'll need to look into this.

Support creating records for `user.starchart.com` #846

uday-rana posted on Feb 24, 2025

To use a custom domain for Amazon Cognito's managed login, the parent domain must have an A record. For example, to use myproject-auth.user.starchart.com, an A record must exist for user.starchart.com. See Amazon Cognito: Using your own domain for managed login

Since this is not supported, it's currently not possible to use a custom domain generated with Starchart for Amazon Cognito. Is this something we can look into?

View on GitHub

React hydration errors: We've known for a minute that sometimes the CSS will fail to load and I discovered this is because of a series of React hydration errors. Still not sure what causes this so we'll have to investigate.

React hydration errors #845

uday-rana posted on Feb 24, 2025

Ran into these errors while using the website. They persisted as I navigated between pages. A hard reload (Ctrl + R) made them go away.

https://react.dev/errors/418?invariant=418 https://react.dev/errors/423?invariant=423 https://react.dev/errors/425?invariant=425

View on GitHub

Other stuff

I also want to try and contribute to chatcraft.org, which some other members of my team have been working on. I found this issue which seems like a good place to start:

Refactor Code To Use `useChat()` Hook #835

mulla028 posted on Feb 20, 2025

Description

Since @humphd landed #822, introduced ChatProvider & useChat() hook. We need to refactor code to stop passing chat on props in components and utilize useChat() hook wherever it's possible .

View on GitHub

Trying to Upgrade from Classic Remix Compiler to Remix Vite

Uday Rana — Fri, 21 Feb 2025 03:46:37 +0000

I've been trying to upgrade our project Starchart from the Classic Remix Compiler to Remix Vite. We recently upgraded from Remix v1 to v2 and that went pretty smoothly, but this jump is proving pretty tricky. I've run into a number of challenges.

Switching from `remix watch` to `remix dev`

When we upgraded to Remix v2 we overlooked switching from remix watch to remix dev. I had to work on that first before I could begin upgrading to Remix Vite.

Upgrade `remix watch` to `remix dev` #842

uday-rana posted on Feb 20, 2025

This is an intermediate step in upgrading to Remix Vite.

Changes

Upgrade remix watch to remix dev
- We were supposed to do this when upgrading to v2
Use tsx to run server.ts and watch for changes instead of building it first with esbuild
- Remix examples use a .js file for the server so I went to the Remix Discord server and they recommended this method. However, this is slower than building the server.
- We no longer need predev, dev:build, delay, dev:remix, dev:server:delay, dev:server scripts
Remove purgeRequireCache
- According to the video walkthrough linked below this is no longer required.
Remove --require ./node_modules/dotenv/config
- According to the video walkthrough linked below, this is no longer required because Remix automatically loads .env files.
Replace ts-node with tsx in prisma.seed
- We already had ts-node to run TypeScript files, but Remix recommends tsx and there's no reason to keep both

References

Upgrading to v2: remix dev: Custom app server @remix-run/dev CLI: remix dev: With custom app server Migrating your project to v2_dev 🚚

View on GitHub

Custom server built with TypeScript

We're using a custom server written in TypeScript, but all of the examples in the documentation for custom servers are written in JavaScript. There's a footnote that mentions using tsx or tsm which I only saw after I'd already switched over to tsx after looking through Remix's Discord server.

I had to add ViteDevServer to our custom server, but the type definitions for ViteDevServer are incompatible with Remix. ViteDevServer.ssrLoadmodule returns Record<string, any> while createRequestHandler({build}) expects an argument of type ServerBuild, so I've had to do some weird assertion stuff.

  app.all(
    '*',
    createRequestHandler({
      build: viteDevServer
        ? async () => {
            const mod = await viteDevServer.ssrLoadModule('virtual:remix/server-build');
            return mod as unknown as ServerBuild;
          }
        : async () => {
            const mod = await import('./build/server/index.js'); // This import's been giving me some trouble too
            return mod as unknown as ServerBuild;
          },
      getLoadContext,
    })
  );

Dynamic import problem

await import('./build/server/index.js');

The dynamic import above is where the server imports all of the server-side code after it's been built. Because of this, the linter and type check are failing in CI, since the module being imported hasn't been built when they're run. I haven't had time to think about the fix - maybe just add an ignore comment - but I feel like there has to be a better solution.

CommonJS

We're also compiling our custom server to CommonJS which there aren't any examples for in the Remix docs. All of the examples for Remix Vite use top-level await, which isn't allowed when compiling to CommonJS so I've had to wrap the whole thing in an IIFE. Maybe switching to ES modules would be a better approach?

There was also this package @emotion/cache that wouldn't import properly until I used a Vite plugin for CommonJS/ES modules interoperability.

// vite.config.ts
import { vitePlugin as remix } from '@remix-run/dev';
import { defineConfig } from 'vite';
import tsconfigPaths from 'vite-tsconfig-paths';
import { cjsInterop } from 'vite-plugin-cjs-interop'; // This plugin

export default defineConfig({
  plugins: [
    !process.env.VITEST &&
      remix({
        ignoredRouteFiles: ['**/.*', '**/*.css', '**/*.test.{js,jsx,ts,tsx}'],
        serverModuleFormat: 'cjs',
      }),
    tsconfigPaths(),
    // https://remix.run/docs/en/main/guides/vite#esm--cjs
    cjsInterop({
      dependencies: ['@emotion/cache'],
    }),
  ],
  optimizeDeps: {
    include: ['@emotion/cache'],
  },
});

Vitest

Also our project is currently on Vitest 0.34 which at one point was causing type errors with Vite 6. I upgraded Vitest to v3 on my branch and fixed the breaking changes and as far as I can tell it's working fine. I just had to switch from watchExclude to server.watch.ignored:

// vitest.config.ts

// Before

/// <reference types="vitest" />
/// <reference types="vite/client" />

import react from '@vitejs/plugin-react';
import { defineConfig } from 'vitest/config';
import tsconfigPaths from 'vite-tsconfig-paths';

export default defineConfig({
  plugins: [react(), tsconfigPaths()],
  test: {
    globals: true,
    setupFiles: ['./test/unit/setup-test-env.ts'],
    include: ['./test/unit/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}'],
    watchExclude: ['.*\\/node_modules\\/.*', '.*\\/build\\/.*', '.*\\/docker\\/volumes\\/.*'],
    coverage: {
      provider: 'istanbul',
    },
  },
});

// After

import react from '@vitejs/plugin-react';
import { defineConfig } from 'vitest/config';
import tsconfigPaths from 'vite-tsconfig-paths';

export default defineConfig({
  plugins: [react(), tsconfigPaths()],
  test: {
    globals: true,
    setupFiles: ['./test/unit/setup-test-env.ts'],
    include: ['./test/unit/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}'],
    coverage: {
      provider: 'istanbul',
    },
  },
  server: {
    watch: {
      ignored: ['.*\\/node_modules\\/.*', '.*\\/build\\/.*', '.*\\/docker\\/volumes\\/.*'],
    },
  },
});

Why bother?

We're considering migrating to React Router v7 and this is an intermediate step. From what I can tell the jump from Remix Vite to React Router v7 should be simpler than the jump from Classic Remix Compiler to React Vite.

I'm also a fan of how much cleaner our npm scripts are (or will be, if I can get this to work).

// Before
    "build": "run-s build:*",
    "build:remix": "remix build",
    "build:server": "esbuild --platform=node --format=cjs ./server.ts --outdir=build --bundle",
    "predev": "npm run build:server",
    "dev": "cross-env NODE_ENV=development SECRETS_OVERRIDE=1 run-p dev:remix dev:server:delay",
    "dev:build": "npm run build:server -- --watch",
    "delay": "node -e \"setTimeout(() => process.exit(0), 3000)\"",
    "dev:remix": "remix watch",
    "dev:server:delay": "run-s delay dev:server",
    "dev:server": "node --inspect --require ./node_modules/dotenv/config ./build/server.js",

// After
    "dev": "cross-env SECRETS_OVERRIDE=1 tsx --inspect ./server.ts",
    "build": "remix vite:build",
    "start": "cross-env SECRETS_OVERRIDE=1 NODE_ENV=production node ./build/server/index.js",

As of now, the dev script and hot module reloading work, the build script works, and the app starts, so all hope is not lost.

Upgrade Classic Remix Compiler to Remix Vite #843

uday-rana posted on Feb 20, 2025

WIP

References

Remix Vite: Migrating How to Fix CJS/ESM Bugs in Remix

View on GitHub

Other stuff this week

Other stuff I worked on this week included:

Improving our contributing documentation

Rework CONTRIBUTING.md #836

uday-rana posted on Feb 14, 2025

Changes

Add unit testing instructions
Add end-to-end testing instructions
Add contributing workflow section
Add resources section
Add separate section with link to wiki to make it more prominent and easier to find
Split setup commands into separate codeblocks for easy copy pasting
Add more headings and reword some sections for clarity
Move testing section directly underneath setup section
Remove npm prerequisite listing since it comes with Node
Remove table of contents since GitHub has one baked-in

View on GitHub

Cleaning up our ignore files (.prettierignore, .dockerignore, .gitignore)

Clean up ignore files #835

uday-rana posted on Feb 14, 2025

Changes

Sort alphabetically
Remove entries that don't exist (like /e2e)
Add new entries

View on GitHub

Improving type handling for DNS Record types

Improve DNS record type handling #837

uday-rana posted on Feb 15, 2025

Follow-up on #830.

In #830, there was discussion on using a function to map type to RRType instead of using type as RRType: https://github.com/DevelopingSpace/starchart/pull/830#pullrequestreview-2613283520

Earlier in the code, we're already asserting type as DnsRecordType, which is a subset of RRType. I think it makes sense to be consistent and use type as DnsRecordType here too.

Also in services/reconciler/route53-client.server.ts > getDnsRecordSetPage(), we can set the type of the type parameter to RRType so we don't need to map it later.

Unrelated changes:

Simplified the logic for handling multi-value records to blacklist types that disallow them (there are only two) instead of whitelisting types that allow them, of which new ones have been added to Route53.
Cleaned up imports in services/reconciler/route53-client.server.ts

View on GitHub

Adding ESLint 9 back after we removed it a while ago, and configuring it to work together with oxlint

Add ESLint 9 to support oxlint #834

uday-rana posted on Feb 13, 2025

Fixes #803

Changes

Added ESLint 9, which is to be run after oxlint as a fallback for linting rules that aren't covered by oxlint. Rules that oxlint does cover won't be linted again by ESLint. This is the approach recommended by oxc:

We recommend running oxlint before ESLint in your lint-staged or CI setup for a quicker feedback loop, considering it only takes a few seconds to run on large codebases.
- I created the config file by grabbing the ESLint 8 flat config template recommended by Remix and using ESLint's config migrator on it. I then tweaked it from there.
Added two oxc plugins not enabled by default: jsx-a11y and import
- These plugins don't implement all of the linting rules of their ESLint equivalents, so we need to keep the ESLint plugins too. See https://github.com/cockpit-project/cockpit/issues/20279
Removed some settings from .oxlintrc.json that didn't do anything or were just re-defining the defaults.
Fixed lint errors detected after ESLint was added.

View on GitHub

Upgrading from Remix v1 to v2

Uday Rana — Thu, 13 Feb 2025 15:40:30 +0000

Last night I worked on upgrading our project Starchart from Remix v1 to v2.

Update Remix to v2 #833

uday-rana posted on Feb 13, 2025

Fixes #808

Changes

Switch from ThrownResponse to ErrorResponse
Switch from @remix-run/node's Response to native Response
- Use Blob to send the body as part of Response
Switch from LoaderArgs to LoaderFunctionArgs
Switch from ActionArgs to ActionFunctionArgs
Switch from V2_MetaFunction to MetaFunction
Switch from useActionData(); to useActionData<typeof action>();
- This is the method used in the docs.
Assert type of route.data as Record<string, unknown> | undefined in useMatchesData()
- This is the method used in this example which is linked to in the docs.

View on GitHub

It was mostly pretty straightforward, thanks to most future flags being already enabled, which meant many breaking changes had already been adopted.

I followed a "2-minute walkthrough" linked in the upgrade documentation, which led to me trying to upgrade the whole thing at once instead of incrementally adopting the remaining feature flags. That aside the walkthrough was actually pretty helpful.

The upgrade was pretty standard stuff - changing some variable names and such - but there were a couple things took me a minute.

Adding <typeof action> to useActionData() to make it useActionData<typeof action>(): I guess this wasn't a requirement in v1 even though the docs used <typeof action>, so upgrading led to type-check errors. I took a quick look at the docs and added that bit there.
Type-casting useMatches(): The type-cast in the code below (as) wasn't in place when we were using v1 and upgrading led to type-check errors. I looked at an example from the docs for useMatches() and saw they used type-casting so I added it here too, and it fixed the type errors.

const matchingRoutes = useMatches();
useMemo(
    () => matchingRoutes.find((route) => route.id === id),
    [matchingRoutes, id]
  ) as { // This bit here
    data: Record<string, unknown> | undefined;
  };

Mobile Safari browser ReadableStream bug: This one took me a while and I wouldn't have figured it out without AI. One of the breaking changes when moving to v2 required switching from @remix-run/node's Response to the native Response. To do this I had to convert the body object passed to the Response to a ReadableStream. According to the Remix docs the normal way of doing this is using PassThrough and createReadableStreamFromReadable():

const body = new PassThrough();
new Response(createReadableStreamFromReadable(body), {
                headers: responseHeaders,
                status: responseStatusCode,
              })

So when I had to update this section of code, I figured I should use PassThrough too.

// Before
function createResponse(body: string | null, filename: string) {
  if (!body) {
    throw new Response('Unable to get certificate part', { status: 400 });
  }

  return new Response(body, {
    status: 200,
    headers: {
      'Content-Type': 'application/x-pem-file',
    }
  });
}

// After
function createResponse(body: string | null, filename: string) {
  if (!body) {
    throw new Response('Unable to get certificate part', { status: 400 });
  }

  const readable = new PassThrough();
  readable.push(body);
  const stream = createReadableStreamFromReadable(readable);

  return new Response(stream, {
    status: 200,
    headers: {
      'Content-Type': 'application/x-pem-file',
      'Content-Disposition': `attachment; filename=${filename}`,
    },
  });
}

But on Mobile Safari, this doesn't work. Something to do with how Safari implements ReadableStream. I saw the implementation tests broke and was able to figure out that this was the change that broke them, since it was the Download test that was failing and this was the only change to the download logic. But I only figured out the actual reason thanks to AI.

I fixed this by switching to a Blob, which has better compatibility with Mobile Safari.

// After After
function createResponse(body: string | null, filename: string) {
  if (!body) {
    throw new Response('Unable to get certificate part', { status: 400 });
  }

  const blob = new Blob([body], { type: 'application/x-pem-file' });

  return new Response(blob.stream(), {
    status: 200,
    headers: {
      'Content-Type': 'application/x-pem-file',
      'Content-Disposition': `attachment; filename=${filename}`,
    },
  });
}

Interesting bug.

Coming Up Next

Next I want to try and migrate from the classic Remix compiler to Remix Vite and then to React Router v7.

Rust? Never Heard of It. Anyway, Here’s My PR.

Uday Rana — Thu, 06 Feb 2025 03:47:26 +0000

Last week I signed up for an issue in Hurl, an HTTP request client and testing tool written in Rust.

--cookie-jar FILE doesn't work if intermediate directories don't exist #3637

jcamiel posted on Jan 24, 2025

When exporting cookies from a run to SOME_FILE export is failing if SOME_FILE has non-existing directory:

$ echo 'HEAD https://hurl.dev' | hurl --cookie-jar /tmp/a/b/c/out.txt
error: Issue writing to /tmp/a/b/c/out.txt: Os { code: 2, kind: NotFound, message: "No such file or directory" }

--cookie-jar should works like --curll which create intermediate directories:

$ echo 'HEAD https://hurl.dev' | hurl --curl /tmp/a/b/c/out.txt

Note: we could try while fixing this bug to mutualize the code that create a file and it's indermediate directories:

 // We ensure that parent folder is created.
    if let Some(parent) = filename.parent() {
        match std::fs::create_dir_all(parent) {
            Ok(_) => {}
            Err(err) => {
                return Err(ReportError::from_error(
                    err,
                    filename,
                    "Issue writing TAP report",
                ))
            }
        }
    }
    let mut file = match File::create(filename) {
        Ok(f) => f,
        Err(e) => {
            return Err(ReportError::from_error(
                e,
                filename,
                "Issue writing TAP report",
            ))
        }
    };

View on GitHub

I needed to fix a command that accepted a file path so that it would create the parent directories in the path if they didn't exist.

My knowledge of Rust is rudimentary at best - if you asked me a week ago I would've told you it was nonexistent. When I started working on this my immediate thought process was to go and "learn Rust" - as in go read the Rust book. I was in the middle of reading what an integer was when I realized it probably wasn't the best use of my time.

Thinking back to my previous experience working in unfamiliar languages and with unknown technologies, I figured I probably don't necessarily need to "know Rust" to be able to contribute. I worked with Go a little bit last year without needing to go read the Go documentation, so I figured I might as well do what I did last time and just jump in.

Once I started actually reading the code, I was able to leverage fundamental programming concepts - I could look at the code and understand I was looking at a function, with type annotations for return types. I could understand I was looking at error handling logic, even though the syntax was quite different from what I'm used to in JavaScript-land, where I usually live.

Even though my first reaction was "Why did I sign up for this?", with that approach, the linter, the unit tests, and some pointers from one of the maintainers, I was able to get the job done.

According to the issue, the logic for this already existed elsewhere in the source code, so I just had to find it and add it to the function that handled the command. Here's the snippet I used:

// We ensure that parent folder is created.
if let Some(parent) = filename.parent() {
    match std::fs::create_dir_all(parent) {
        Ok(_) => {}
        Err(err) => {
            return Err(CliError::IO(format!(
                "Issue writing to {}: {err:?}",
                filename.display()
            )));
        }
    }
}

Create parent folders if missing when using --cookie-jar FILE #3688

uday-rana posted on Feb 03, 2025

Fixes #3637

Please let me know if there's anything I could improve. Thanks.

View on GitHub

In addition to the fix, the maintainer also asked me to do some refactoring, by extracting the parent directory creation logic into a util function and replacing the existing logic with that function throughout the source code. They helped me out by providing some snippets of how they'd prefer it to be done, and by this point I was a little bit more comfortable reading and writing Rust, so it wasn't too bad.

Here's the same logic but as a function:

// Create parent directories, if missing, given a filepath ending with a file name
pub fn create_dir_all(filename: &Path) -> Result<(), std::io::Error> {
    if let Some(parent) = filename.parent() {
        return std::fs::create_dir_all(parent);
    }
    Ok(())
}

You might notice this function is calling a standard library function with the same name. That threw me off at first - why was I writing a function when there already is one that does the same thing? The difference is that the standard library function requires a directory path as the argument and doesn't work when it ends in a file name, while this function wraps around it and strips the file name before calling it.

With that in place, my changes were accepted. Would I feel confident putting Rust on my resume? Probably not (yet). But it was fun to explore nevertheless.

My Plans In Open Source This Week

Uday Rana — Sun, 02 Feb 2025 16:20:18 +0000

This is a report on what I plan to work on this week in open source. I signed up for work on two issues.

Hurl: `--cookie-jar FILE` doesn't work if intermediate directories don't exist

The first is an issue in Hurl, an HTTP request/response client and assertion tool.

--cookie-jar FILE doesn't work if intermediate directories don't exist #3637

jcamiel posted on Jan 24, 2025

When exporting cookies from a run to SOME_FILE export is failing if SOME_FILE has non-existing directory:

$ echo 'HEAD https://hurl.dev' | hurl --cookie-jar /tmp/a/b/c/out.txt
error: Issue writing to /tmp/a/b/c/out.txt: Os { code: 2, kind: NotFound, message: "No such file or directory" }

--cookie-jar should works like --curll which create intermediate directories:

$ echo 'HEAD https://hurl.dev' | hurl --curl /tmp/a/b/c/out.txt

Note: we could try while fixing this bug to mutualize the code that create a file and it's indermediate directories:

 // We ensure that parent folder is created.
    if let Some(parent) = filename.parent() {
        match std::fs::create_dir_all(parent) {
            Ok(_) => {}
            Err(err) => {
                return Err(ReportError::from_error(
                    err,
                    filename,
                    "Issue writing TAP report",
                ))
            }
        }
    }
    let mut file = match File::create(filename) {
        Ok(f) => f,
        Err(e) => {
            return Err(ReportError::from_error(
                e,
                filename,
                "Issue writing TAP report",
            ))
        }
    };

View on GitHub

This issue concerns the command --cookie-jar <FILE>. If the file path specified contains intermediate directories that doesn't exist, it throws an error. We want it to automatically create the intermediate directories.

Hurl is a tool that I use (and still use) in my own projects. I want to continue doing meaningful work and I thought this would be a good way to get back into the flow of things since I haven't done much open source work since December. This will also be my first time working with Rust, which I'm looking forward to.

Starchart: Add ability to create and manage MX records

The second issue I signed up for is an issue in Starchart, our in-house tool for creating and managing custom subdomains.

Add ability to create and manage MX records #787

humphd posted on Jan 24, 2025

Currently you can create the following types of DNS records:

A
AAAA
CNAME
TXT

A request has come in to support also including MX records for defining names for email hosts.

View on GitHub

We want to add support for MX records, a type of DNS record for mail servers. I'm not too familiar with how the app manages DNS records so I'll have to do some research. This issue seems rather large in scope, so while I will be working on the implementation, I don't expect to finish it within the week. My professor also suggested I split it up into smaller issues - writing tests and doing the actual implementation - and let somebody else do the other half.

I signed up for this because we've been updating dependencies in this project for a couple weeks now to bring it up to date, but haven't had a chance to dive into the actual code, so I thought it'd be nice to finally do just that.

I did do some other minor stuff like cleaning up .prettierignore, adding an .nvmrc file, adding docker compose down to the contributing docs, and organizing the project structure a little.

Remove .prettierignore entries that are already in .gitignore #812

uday-rana posted on Jan 31, 2025

Fixes #811

View on GitHub

Add `docker compose down` to contributing docs #809

uday-rana posted on Jan 30, 2025

Fixes #771

View on GitHub

Add .nvmrc #806

uday-rana posted on Jan 30, 2025

Fixes #773

View on GitHub

Move docs into /docs #810

uday-rana posted on Jan 31, 2025

I think it'd be nice to clean the project root directory up to make it a little easier to navigate. This moves the documentation and images into /docs so they're not mixed in with config files and such.

View on GitHub

Move mysql and redis bind mounts to /docker/volumes #813

uday-rana posted on Jan 31, 2025

This aims to make the project root directory a bit more organized by moving all bind mounts under a single directory.

View on GitHub

I'll do a progress report on Wednesday with how the issues are going.

Reviving An Outdated Project

Uday Rana — Tue, 21 Jan 2025 21:36:30 +0000

This week, I did some some maintenance work on Starchart. The project hasn't been worked on in a while so we're trying to update it's dependencies.

DevelopingSpace / starchart

A self-serve tool for managing custom domains and certificates

Starchart

Starchart makes it easy for the Seneca developer community to create and manage their own custom subdomains and SSL certificates, without cost or having to provide personal information.

For information about running Starchart, see our deployment guide. For development information, see our contributing guide. For further technical background, planning, and initial designs, please see the wiki.

Introduction

The internet is evolving, and what used to be hard has become simple. For example, hosting your own website used to require knowledge of server administration, operating systems, networking, etc. Today, many developers host their personal and project websites without ever touching a remote server, opting for (free) cloud services like GitHub Pages, Vercel, Netlify, or AWS.

The internet's security model is also evolving. For example, browser vendors have embraced HTTPS everywhere. This is good for security, as it enables certificate-based encryption between clients and servers. However, as with…

View on GitHub

The plan was to fix the CI workflow, which we found out was broken last week:

But before I could work out a fix, one of the previous developers, Eakam, solved the issue - turns out it was just because Playwright was outdated.

Bump playwright to 1.49.1 #772

Eakam1007 posted on Jan 18, 2025

Playwright install is failing in CI (E2E Tests). Bumping playwright version should fix that.

Ref:

View on GitHub

I felt like I should make up for it by finding more stuff to work on and thought updating more dependencies would be a great starting point.

Since the project hadn't been worked on for 2 years, there were a bunch of security vulnerabilities stemming from outdated packages. I was able to fix most of them with npm audit fix.

There were a couple more fixes that led to breaking changes in @remix-run/eslint-config and @remix-run/react, so I bumped those manually.

One of the updates (I bumped them at the same time so I can't say for sure but my bet is on /react) led to a type-check error because [@remix-run/react].useNavigation().formData may now be of the type undefined. I fixed it with optional chaining.

// Before
const isLoading =
  navigation.state === 'submitting' &&
  Number(navigation.formData.get('id')) === dnsRecord.id;

// After
const isLoading =
  navigation.state === 'submitting' &&
  Number(navigation.formData?.get('id')) === dnsRecord.id;

The other changes I made had to do with some lint errors that popped up (At this point I realized I had my ESLint extension turned off, but I'm sure these warnings came with the update, since it never happened in CI in the past).

Instances of importing the same module multiple times in one file:

// Before
import { getCertificateByUsername } from '~/models/certificate.server';
import { deleteCertificateById } from '~/models/certificate.server';
import { isAdmin } from '~/models/user.server';
import { getUserByUsername } from '~/models/user.server';

// After
import { getCertificateByUsername, deleteCertificateById } from '~/models/certificate.server';
import { isAdmin, getUserByUsername } from '~/models/user.server';

Using let when const is preferred:

// Before
let date = val.toLocaleDateString('en-US', {

// After
const date = val.toLocaleDateString('en-US', {

Surprised it didn't catch these before.

Also, when I turned on the ESLint extension I was a little taken aback because there were ~900 linter errors. Turned out it was because ESLint was linting the output generated by Playwright. So I added /playwright-report to .eslintignore.

And that was the sum of my maintenance work for this sprint. Ended up fixing 30+ severe security issues, so not bad.

Update dependencies #775

uday-rana posted on Jan 18, 2025

Should fix a bunch of security vulnerabilities.

Changes

[x] Bump dependencies
[x] Add /playwright-report to .eslintignore
[x] Fix typecheck and linter errors

View on GitHub

I also re-activated Dependabot which bumped vitest a couple minor versions. It'll be nice to not have to manually investigate and patch security vulnerabilities.

In other news, one of my pull requests to Mattermost was finally merged!

[GH-29548] Avoid SELECT * in `tokens_store.go` #29558

uday-rana posted on Dec 10, 2024

Summary

This PR:

Switches SQL queries in token_store.go to use SQLBuilder
Explicitly defines columns in SELECT queries to TokenStore.
Factors out common queries into the constructor.

Ticket Link

Fixes #29548

Screenshots

Release Note

NONE

View on GitHub

It'd been approved a while ago but it took a few weeks to be merged into main.

In the meantime I've been working on my other PR. I was asked to make some changes and I'm waiting on a re-review.

[MM-53650] Add disable emoticon rendering setting to webapp #29414

uday-rana posted on Nov 28, 2024

Summary

This pull request adds a user setting to the webapp to toggle rendering emoticons (:D) as emojis (😄).

The setting is added as a component in components/user_settings/display/render_emoticons_as_emoji/ which is imported in components/user_settings/display/user_settings_display.tsx.

I've added a renderOnOffLabel() function to user_settings_display.tsx, lifted from components/user_settings/advanced/user_settings_advanced.tsx to help render the new component.

The setting is stored as a user preference using the savePreferences() action.

I've added constants for the preference to utils/constants.tsx and webapp/channels/src/packages/mattermost-redux/src/constants/preferences.ts.

I've updated affected tests and created unit tests for the new component. I've also updated the English translation file.

QA Test Steps

Navigate to User Settings.
Go to the Display category.
Find the section labelled "Auto-render emoticons as emoji" and click "Edit".
Toggle the setting and click "Save".
Emoticon rendering on messages sent by the current user and other users should be toggled client-side with the setting.

Ticket Link

Fixes (partially) https://github.com/mattermost/mattermost/issues/26504 Jira https://mattermost.atlassian.net/browse/MM-53650

Note the issue and ticket describe adding this feature to the mobile app as well, which this PR does not.

Screenshots

before	after

Release Note

Added a new user setting to toggle rendering emoticons (:D) as emojis (😄)

View on GitHub

Working on this PR was interesting because when I first submitted it I didn't even entirely understand my changes. Going back into it after a long while away and with the feedback from the reviews helped me look at it from a fresh perspective and understand it better.

The Mattermost app gets user setting state from both "preferences" and from a "config". I added my setting to both, mimicking one of the existing settings I was advised to reference, but it turned out the "config" is for server-level settings, while this new setting was intended to be a client-side option. The reviews helped me understand where I went wrong, and it actually ended up being a smaller change than I thought necessary.

Overall I'd say it was a fairly productive week.

Becoming An Open Source Maintainer

Uday Rana — Tue, 14 Jan 2025 23:17:34 +0000

I've had the opportunity to enrol in OSD700 - Open Source Development Project at Seneca Polytechnic, where I'll be learning to maintain and ship open source software as part of a team. As a part of the course, I'll be blogging weekly about everything I work on.

This term, we're going to be maintaining two projects: Starchart and ChatCraft.

DevelopingSpace / starchart

A self-serve tool for managing custom domains and certificates

Starchart

Starchart makes it easy for the Seneca developer community to create and manager their own custom subdomains and SSL certificates, without cost or having to provide personal information.

Introduction

View on GitHub

tarasglek / chatcraft.org

Developer-oriented ChatGPT clone

ChatCraft.org

Features

We think ChatCraft is the best platform for learning, experimenting, and getting creative with code. Here's a few of the reasons why we think you'll agree:

🛠️ You're in Control: Customize all aspects of a chat. Use your own System Prompts, edit, delete, and retry AI messages with models from competing vendors in the same chat.

🌍 Multiple AI Providers: ChatCraft supports both OpenAI and OpenRouter, giving you access to a…

View on GitHub

I'll talk about my experience setting these up, and what I plan to work on for this sprint.

Setting up the projects

I was able to get both projects up and running fairly easily following the documentation. Starchart is an old project and hasn't been maintained for about two years, so I got warnings for a bunch of out-of-date dependencies, but was still able to get the webapp running.

ChatCraft is much more actively maintained, so I didn't run into any trouble setting it up.

First Contributions

I submitted some issues / pull requests for both projects - some feature suggestions and some documentation updates.

Starchart

For Starchart, I noticed a few areas for improvement in the docs:

The contributing docs don't mention docker compose down, so contributors unfamiliar with Docker Compose might end up leaving the Docker services on their machines.

Add `docker compose down` to contributing docs #771

uday-rana posted on Jan 15, 2025

The contributing docs don't mention docker compose down, so contributors unfamiliar with Docker Compose might end up leaving the containers on their machines.

View on GitHub

The section on squashing and merging pull requests has some misleading info saying squashing helps prevent losing the commit history.

I think we could write a Makefile to simplify the developer environment setup and teardown.

Simplify dev environment setup with a Makefile #770

uday-rana posted on Jan 15, 2025

I think we can simplify the dev environment setup using a Makefile so that a single command can run all the necessary scripts:

https://github.com/DevelopingSpace/starchart/blob/bf23d1c5e84298165463e34cb337f00aeb07f862/CONTRIBUTING.md?plain=1#L37-L63

View on GitHub

I made a pull request to fix some typos and the section on merging pull requests. Unfortunately, it looks like Starchart's CI workflow is broken, because Playwright fails to install the dependencies it needs to run end-to-end tests. I haven't been able replicate this error on my local machine - I assume it's because of differences between my environment and the GitHub Actions environment. I'll have to look into the CI workflow.

Fix typos and "Merging to main" section in CONTRIBUTING.md #768

uday-rana posted on Jan 09, 2025

Fixes #767.

Fixed some typos. Also:

https://github.com/DevelopingSpace/starchart/blob/bf23d1c5e84298165463e34cb337f00aeb07f862/CONTRIBUTING.md?plain=1#L122-L129

This section says that squashing and merging pull requests helps prevent losing the commit history which is not true. I've streamlined the section to remove that part.

View on GitHub

Until this is resolved I've decided to hold off on making pull requests for the other two issues.

ChatCraft

As for ChatCraft, the Getting Started section of it's README had what I understood to be slightly outdated instructions, instructing new users to select an AI provider and to enter an API Key. These are optional steps tucked away in the settings, a fact the instructions failed to mention (which confused me when trying it out the first time). I filed an issue to address this.

README "Getting Started" section improvements #776

uday-rana posted on Jan 09, 2025

The Getting Started section of the README talks about choosing an AI provider and entering an API key, but fails to mention where to find these settings and that these steps are optional:

https://github.com/tarasglek/chatcraft.org/blob/747d1026e3eee0ca3db829b2f09fe9ded42b83e2/README.md?plain=1#L33-L42

We could either remove those steps for the sake of simplicity (since it is supposed to be "Getting Started"), or label them as optional and mention where users can find those options.

View on GitHub

I also opened an issue for adding a setting which updates the app theme dynamically when the system theme changes, instead of reading the system theme on start and then requiring manual toggling.

Add "Use System Theme" setting #781

uday-rana posted on Jan 14, 2025

I'd like a setting that makes the app theme change dynamically when the system theme changes.

I think setting useSystemColorMode to true if the setting is toggled on would work:

https://github.com/tarasglek/chatcraft.org/blob/c2f7165a7225492dcd1c1ef18e65272e3f5f59ab/src/theme.ts#L5-L10

Maybe also hide/disable the theme toggle button when this is on?

View on GitHub

Other Plans For This Sprint

In terms of other stuff for this sprint, I'm still working on one of my pull requests to Mattermost from a while ago - I received a review last week so I have some changes to make:

[MM-53650] Add disable emoticon rendering setting to webapp #29414

uday-rana posted on Nov 28, 2024

Summary

This pull request adds a user setting to the webapp to toggle rendering emoticons (:D) as emojis (😄).

The setting is added as a component in components/user_settings/display/render_emoticons_as_emoji/ which is imported in components/user_settings/display/user_settings_display.tsx.

I've added a renderOnOffLabel() function to user_settings_display.tsx, lifted from components/user_settings/advanced/user_settings_advanced.tsx to help render the new component.

The setting is stored as a user preference using the savePreferences() action.

I've added constants for the preference to utils/constants.tsx and webapp/channels/src/packages/mattermost-redux/src/constants/preferences.ts.

I've updated affected tests and created unit tests for the new component. I've also updated the English translation file.

QA Test Steps

Navigate to User Settings.
Go to the Display category.
Find the section labelled "Auto-render emoticons as emoji" and click "Edit".
Toggle the setting and click "Save".
Emoticon rendering on messages sent by the current user and other users should be toggled client-side with the setting.

Ticket Link

Fixes (partially) https://github.com/mattermost/mattermost/issues/26504 Jira https://mattermost.atlassian.net/browse/MM-53650

Note the issue and ticket describe adding this feature to the mobile app as well, which this PR does not.

Screenshots

before	after

Release Note

Added a new user setting to toggle rendering emoticons (:D) as emojis (😄)

View on GitHub

Looking Back On My Work

Uday Rana — Thu, 12 Dec 2024 05:36:34 +0000

In this post I'll be reviewing my progress in open source over the past few weeks.

Originally, my plan for this past month or so was to work on Mattermost Mobile, where I was going to add a feature to toggle rendering emoticons as emojis. I worked on the same feature in the webapp, so I figured I should have a solid idea of what I needed to do. Quick update on the webapp pull request by the way, my changes were finally reviewed by the contributors. I'll be working on making the requested changes over the next few days.

Anyway, unfortunately, I ended up being way busier than I could've anticipated and I wasn't able to start on the mobile issue until very recently, where I immediately hit a roadblock in not being able to set up the dev environment due to my operating system, meaning I couldn't do the work I was planning on, and had to change gears fast to still get contributions in. I decided my best option was to look for smaller, but still substantial issues.

I looked through GitHub for issues labelled "help wanted", and ended up finding a couple more Mattermost issues, along with one for the GitHub CLI:

Issue 1: Github CLI - Improving output formatting

`gh run view` should list branches in square brackets #10038

BagToad posted on Dec 07, 2024

Describe the bug

gh run view lists branches in parenthesis, but I think it should list them in square brackets to align with Primer guidelines:

Display branch names in brackets and/or cyan

Steps to reproduce the behavior

gh run view

Expected vs actual behavior

gh run view prompts should display branches within square brackets.

Logs

❯ gh run view
? Select a workflow run  [Use arrows to move, type to filter]
> - Verifying attestations offline fails, Discussion Triage (trunk) 4h55m1s ago
  - Decoding, Discussion Triage (patch-1) 4h59m32s ago
  ✓ Decoding, PR Automation (patch-1) 4h59m43s ago
  ✓ Issue Automation, Issue Automation (trunk) 5h20m31s ago
  - `gh repo rename myorg/newname` results in `myorg/myorg-newname`, Discussion Triage (trunk) 10h13m50s ago
  - 401 Error at every turn, Discussion Triage (trunk) 10h15m20s ago
  - 401 Error at every turn, Discussion Triage (trunk) 10h15m20s ago

View on GitHub

Issue 2: Mattermost - Adding timestamps to pasted image filenames

Add timestamp to pasted image filenames to differentiate them #29524

mattermod posted on Dec 09, 2024

The Problem

When pasting an image from the clipboard into the message input, it gets the filename image.png. If multiple images are pasted, they all get the same image.png filename. This can be a problem when downloading each file with the same name with danger of overwriting.

Proposed solution

Auto-generate a Filename that includes the timestamp with this format:

image-2024-11-18-6-29-57-PM

Mattermost thread: https://hub.mattermost.com/private-core/pl/xryg3tedg3bbxq3xuwnrj9ymyc

If you're interested please comment here and come join our "Contributors" community channel on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers" community channel.

New contributors please see our Developer's Guide.

JIRA: https://mattermost.atlassian.net/browse/MM-62003

View on GitHub

Issue 3: Mattermost - Improving SQL query maintainability and backward compatibility

Avoid SELECT * in tokens_store.go #29548

mattermod posted on Dec 10, 2024

SELECT * makes ADD COLUMN a backwards incompatible schema change: older servers running against a newer schema won’t know what to do with the new columns.

This ticket tracks the effort required to migrate the SQL queries in https://github.com/mattermost/mattermost/blob/master/server/channels/store/sqlstore/tokens_store.go . (Please submit a single PR with changes to only this store.)

Step 1: Migrate any SELECT * queries written as strings into builder queries.

First, if the query is hard-coded as a string, e.g. SELECT * FROM Table WHERE ..., migrate this query to using the SQLBuilder. So instead of this:

if _, err = s.GetReplica().Get("SELECT * FROM Table WHERE Id = ?", id); err != nil {
    return err
}

change it to this instead:

query := s.getQueryBuilder().
    Select("*").
    From("Table").
    Where(sq.Eq{"Id": id})

if _, err = s.GetReplica().GetBuilder(query); err != nil {
    return err
}

It’s not necessary to change all queries in this [file: jus|file: ju]t the ones doing a SELECT *.

Step 2: Migrate any instances of SELECT * into explicitly enumerating the needed columns

Instead of SELECT("*"), rewrite to explicitly specify the columns needed:

query := s.getQueryBuilder().
    Select("Column1", "Column2").
    From("Table").
    Where(sq.Eq{"Id": id})

If these columns are referenced more than once in the file, factor out the query into the store constructor:

type SqlTableStore struct {
    *SqlStore

    tableSelectQuery sq.SelectBuilder
}

func newSqlTableStore(sqlStore *SqlStore) store.TableStore {
    s := SqlTableStore{
        SqlStore: sqlStore,
    }

    s.tableSelectQuery = s.getQueryBuilder().
        Select("Column1", "Column2").
        From("Table")

    return &s
}

and then extend it everywhere:

query := s.tableSelectQuery.Where(sq.Eq{"Id": id})

Step 3: Run the related unit tests

Within the server/channels/store/sqlstore run the tests for the associated store, e.g. go test -run TestTableStore (replace with the test for the store in question).

New contributors please see our Developer's Guide.

JIRA: https://mattermost.atlassian.net/browse/MM-62144

View on GitHub

How It Went

I'll talk about the issues in order.

Issue 1: Github CLI - Improving output formatting

This issue was based on improving the output formatting for the gh run view subcommand to bring it closer in line with GitHub's design guidelines by displaying branch names in square brackets instead of parentheses.

This issue was a much needed change of pace from the gargantuan issue I was working on for Mattermost. After struggling with the dev environment setup for days and failing, I needed something to get back into the flow of things and this issue was perfect.

Setting up the dev environment was super simple because the only thing I needed to install was Go. I started, as always, by looking at the contributing docs, where I found a nice explanation of what goes in each project directory, which helped me find the file containing the subcommand's logic. I noticed the display function wasn't actually in that file and traced it back to a common utilities file for the run command. From there, I just updated the display function to print square brackets instead of parentheses. I noticed it looked very reminiscent of C's printf, which is apparently because Go is designed after C.

Testing my changes manually was also very straightforward. It was a single command to build the binary, and then I could use it from the terminal. So I made the change and submitted my PR but forgot to run and update unit tests which I then corrected as soon as I realized.

At this point the maintainer who opened the issue informed me that the issue hadn't been triaged yet and they weren't sure if they wanted to actually implement this. Fortunately, they later did agree on the changes, only with the additional acceptance criteria of also updating the formatting for the gh codespace command.

So I made the changes - again, pretty straightforward process (Didn't forget to update the tests this time), and my changes were approved and merged. All in all, a pretty relaxed experience compared to other stuff I've worked on.

Issue 2: Mattermost - Adding timestamps to pasted image filenames

For this issue, the maintainers noticed that images pasted from the clipboard would all receive the name "image.png", and wanted to add timestamps on paste, to help differentiate them in case they overwrite each other when downloading them.

This seemed like it'd be a pretty straightforward issue, but actually ended up being pretty interesting, although me and the maintainers couldn't come to an agreement on the solution.

The issue stemmed from the browser automatically assigning the file name "image.png" to clipboard images. Now, simply adding a check for "image.png" would be stupid simple, but of course it's never that easy. I found this thread on the Atlassian Confluence forums about a very similar issue. Confluence appended timestamps to images with a hardcoded check for the filename "image.png", but things broke when Firefox started localizing the filename, leading to German users ending up with the filename "grafik.png".

Even though I manually tested the localization by switching Firefox to German and wasn't able to reproduce it (i.e. it remained "image.png"), the possibility that the filename could change at all on different platforms/environments meant I couldn't just hardcode the check.

Another problem was that hardcoding the check meant if a user actually had a file named "image.png" we would end up modifying the filename which we didn't want to do. So, we needed a way to differentiate user-defined filenames and automatically generated filenames, without hardcoding them.

Unfortunately, all we had to work with was a ClipboardEvent, which didn't give us enough information to tell the difference. So it seems like the desired solution, where we wouldn't modify filenames unless they were automatically generated was not possible.

I proposed an alternative where we could append timestamps to all files, either on upload on download, maybe as a toggle-able setting, but this doesn't really directly address the issue. I've yet to hear back on what direction the maintainers want to go in, if they still want to implement this at all.

Issue 3: Mattermost - Improving SQL query maintainability and backward compatibility

This issue stands out a bit among other issues I've worked on so far because it finally involves updating backend logic. The idea is to improve database queries by switching from SELECT * to explicitly defining columns, ensuring backwards compatibility with database schema changes (older servers running against a newer schema won’t know what to do with the new columns). In addition, to improve maintainability, the queries were to be migrated from hardcoded strings to being constructed using an SQL query builder API.

When I started working on this issue, I noticed the issue description talked about using a method called getReplica() but that method didn't seem to exist, so I referenced an existing function in the same file that already used the query builder and used getReplicaX().

In order to find out what columns the table actually contained, I needed to go into the database and look around, but I noticed there were two database containers being spun up - MySQL and Postgres. I looked at the docs and saw that Postgres was preferred for new deployments and that there were in-app settings to choose the database, so I went into the settings to take a look and confirmed that Postgres was in fact selected, so I figured I should probably check out the Postgres container. I needed the database username so I went into the Docker Compose file and found an environment variable for it. I then used docker exec to run psql and find the table and it's columns.

In order to figure out query builder syntax for selecting multiple columns, I searched the repo for the string getQueryBuilder( and found an example at server/channels/store/sqlstore/bot_store.go.

I also factored out the common query (SELECT token, createat, type, extra FROM Tokens) into the type and it's constructor like the issue said and modified the other queries to use the newly factored-out query.

So I made the pull request, but I realized the maintainer ended up updating the file I was working on after I'd made my branch, and this led to merge conflicts. Following their contribution guidelines, instead of rebasing I merged master into my branch. Remember how I couldn't find the getReplica() method? They'd updated the file to now use that method. This method had different syntax so I had to go and look for examples so I could update my changes while resolving the merge conflict.

This PR is still pending review.

Final Thoughts

In this process, I was exposed to Go, psql, and Web APIs like ClipboardEvent and DataTransferItem, which made for a fun learning experience. In fact, I ended up working with Go in two out of three of the issues which I thought was interesting. Seems like Go is growing in popularity.

That's about it for this post. This also marks the end of the course I'm taking on Open Source Development, OSD600 at Seneca Polytechnic. It's been a fantastic journey starting from barely understanding how to use Git to being able to use it to effectively contribute to huge projects. The course may be over but I'm certain I'll continue to work in open source in the future. Thanks for reading.

Forem: Uday Rana

Remix Vite migration pains

References

Planning the Last Sprint

Starchart

References

chatcraft.org

Fixing a regression with git bisect

tarasglek / chatcraft.org

Developer-oriented ChatGPT clone

ChatCraft.org

Features

Issue 1

Click on checkbox next to openrouter

Click on openrouter api key box

Bug 1: Broken LLM selection in settings

Description

Link to Reproduction

Chakra UI Version

Browser

Operating System

Bug 2: "Get OpenRouter Key" button only shows when OpenRouter API key field is focused

Issue 2

Description

Extra

Progress Update

chatcraft.org

Description

Mattermost

Summary

QA Test Steps

Ticket Link

Screenshots

Release Note

Back to Work

Pull Requests

Description

References

References

Changes

References

Issues filed

Other stuff

Description

Trying to Upgrade from Classic Remix Compiler to Remix Vite

Switching from remix watch to remix dev

Changes

References

Custom server built with TypeScript

Dynamic import problem

CommonJS

Vitest

Why bother?

References

Other stuff this week

Changes

Changes

Changes

Upgrading from Remix v1 to v2

Changes

Coming Up Next

Rust? Never Heard of It. Anyway, Here’s My PR.

My Plans In Open Source This Week

Hurl: --cookie-jar FILE doesn't work if intermediate directories don't exist

Starchart: Add ability to create and manage MX records

Reviving An Outdated Project

DevelopingSpace / starchart

A self-serve tool for managing custom domains and certificates

Starchart

Introduction

Changes

Summary

Ticket Link

Screenshots

Release Note

Summary

QA Test Steps

Ticket Link

Screenshots

Release Note

Switching from `remix watch` to `remix dev`

Hurl: `--cookie-jar FILE` doesn't work if intermediate directories don't exist