cheatcode / joystick

A full-stack JavaScript framework for building stable, easy-to-maintain apps and websites.
https://cheatcode.co/joystick
Other
211 stars 11 forks source link

Refactor isBrowser and isNode checks inside of buildFiles.js to use a regex instead of .includes() #230

Closed rglover closed 2 months ago

rglover commented 1 year ago

Right now, if we have a file in the tree that includes any of the words here, the build script naively matches those words anywhere in the path. When it does, it forces that file to skip its build and potentially breaks the app.

E.g., with a page path like /ui/pages/apublicpage, the lowercase public matches and instead of the page being built as expected, it just gets copied raw. When we try to load that page, we get a bunch of Node.js resolution errors because the browser can't interpret a path like @joystick.js/ui and bugs out.

See here for what to patch: https://github.com/cheatcode/joystick/blob/development/cli/src/functions/start/buildFiles.js#L66

rglover commented 1 year ago

Got this fixed but in the process, realized the checks for isBrowser and isNode suffer from the same issue (doing matches using string.includes()). The fix is to just use a regex but in this specific case, you have to be careful because we convert the internal paths to be OS specific (e.g., Windows uses backslashes in file paths not forward slashes).

Best to push out the fix for the above and then do the isBrowser isNode conversion later when it can be properly tested.