Skip to content

Conversation

@jansedlon
Copy link
Contributor

Description

Updates all dependencies in this project since some of them were really old, for example vitest, typescript and @vitest/coverage-v8.

What to do with the CJS version of react router app?

I haven't touched the CJS version and it seems like that even if it builds, i cannot run the CJS version.

First of all, the CJS build of Vite's Node API is deprecated even in the version that's defined in the current state (version 5).
Secondly, if I build it and run it, it exits with the following error

pnpm start

> react-router-cjs@ start /Users/jansedlon/Developer/oss-jansedlon/test-apps/react-router-cjs
> react-router-serve ./build/server/index.js

(node:87834) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.
(Use `node --trace-warnings ...` to show where the warning was created)
/Users/jansedlon/Developer/oss-jansedlon/test-apps/react-router-cjs/build/server/index.js:1
import { jsx, jsxs } from "react/jsx-runtime";
^^^^^^

SyntaxError: Cannot use import statement outside a module
    at wrapSafe (node:internal/modules/cjs/loader:1486:18)
    at Module._compile (node:internal/modules/cjs/loader:1528:20)
    at Object..js (node:internal/modules/cjs/loader:1706:10)
    at Module.load (node:internal/modules/cjs/loader:1289:32)
    at Function._load (node:internal/modules/cjs/loader:1108:12)
    at TracingChannel.traceSync (node:diagnostics_channel:322:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:220:24)

Type of change

Please mark relevant options with an x in the brackets.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected) - TS Up now outputs .js and .cjs instead of .mjs and .js. Not sure if it's a breaking change
  • This change requires a documentation update
  • Algorithm update - updates algorithm documentation/questions/answers etc.
  • Other (please describe): Modernizes packages and build output of tsup

How Has This Been Tested?

I have cloned this repo, updated the packages listed in this PR, fixed the lefthook command.
I have also

  1. Ran pnpm build and checked the output of arethetypeswrong
  2. Pushed this commit with git hooks enabled and worked fine.
  • Integration tests
  • Unit tests
  • Manual tests
  • No tests required

Reviewer checklist

Mark everything that needs to be checked before merging the PR.

  • Check if the UI is working as expected and is satisfactory
  • Check if the code is well documented
  • Check if the behavior is what is expected
  • Check if the code is well tested
  • Check if the code is readable and well formatted
  • Additional checks (document below if any)
  • Check if the CI workflow works fine

@jansedlon
Copy link
Contributor Author

tsup just got a successor called tsdown by the rolldown team, should i add it? 😛

@AlemTuzlak
Copy link
Contributor

@jansedlon LGTM, for the CJS question, I guess that might be an issue with the build script or something, as soon as Vite drops CJS instead of deprecating it I'm dropping it here as well, the important part is that the package bundlesp roperly for CJS, the end app not working is a different thing. I won't pay it much attention as it's deprecated anyway

@AlemTuzlak
Copy link
Contributor

For the rolldown stuff, would love to add it!

@pkg-pr-new
Copy link

pkg-pr-new bot commented Apr 21, 2025

Open in StackBlitz

npm i https://pkg.pr.new/forge-42/open-source-stack@14

commit: e653107

@AlemTuzlak AlemTuzlak merged commit b82d84d into forge-42:main Apr 21, 2025
5 checks passed
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