-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Modernize Deno template #9932
base: dev
Are you sure you want to change the base?
Modernize Deno template #9932
Conversation
🦋 Changeset detectedLatest commit: 3907b73 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
5222ff2
to
7d7761a
Compare
7d7761a
to
3daf1b6
Compare
1a780f3
to
aacb0ee
Compare
aacb0ee
to
ead6719
Compare
78be9f6
to
0b86c3d
Compare
export function installGlobals({ | ||
nativeFetch, | ||
}: { nativeFetch?: boolean } = {}) { | ||
if (process.versions.deno) { | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
making installGlobals
a no-op in Deno allows the @remix-run/serve cli to work without issues
Would you mind writing a short explanation on how to test this out locally? I'm clearly not clueing into something and I can't get this to run. |
sure no worries @zkdiff, here's some instructions.
|
"dependenciesMeta": { | ||
"@remix-run/dev": { | ||
"injected": true | ||
}, | ||
"@remix-run/node": { | ||
"injected": true | ||
}, | ||
"@remix-run/react": { | ||
"injected": true | ||
}, | ||
"@remix-run/serve": { | ||
"injected": true | ||
}, | ||
"@remix-run/server-runtime": { | ||
"injected": true | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is necessary for deno as these packages are cjs and not esm. deno will refuse to run userland cjs code which is what happens due to the workspace dependencies in node_modules being symlinked by default with pnpm. dependenciesMeta.injected
tells pnpm to install these workspace dependencies without using symlinks which convinces deno that these are npm packages (as they're now inside the node_modules
folder and not symlinked out of there) and enables cjs for it.
return true; | ||
} | ||
|
||
async function updateDenoJSON(ctx: Context) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this only supports a deno.json
file in a template without comments for simplicity's sake
d836e1e
to
0b65afb
Compare
## Deno | ||
deno -A npm:create-remix@latest --template remix-run/remix/templates/deno |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alternatively this could be npx create-remix@latest --template remix-run/remix/templates/deno --package-manager deno
, but i think using deno here makes more sense as it avoids needing another runtime + package manager just to scaffold.
(process.versions.deno | ||
? "deno" | ||
: // npm, pnpm, Yarn, and Bun set the user agent environment variable that can be used | ||
// to determine which package manager ran the command. | ||
(process.env.npm_config_user_agent ?? "npm").split("/")[0]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The work here is amazing, bumping this MR up to get more eyes on it |
This modernizes the Deno template to use all the goodies of the releases over the past several years. It removes the need for a package.json file, any dependency on Node.js + npm and now uses Vite instead of the classic Remix compiler.
It also drops the need for the
@remix-run/deno
package, allowing it to be deprecated and removed in the future. I am not sure what the deprecation + removal process looks like for a remix package so I have opted to leave that out of this PR.The new Deno template is currently setup to lint the
app/
directory with eslint (which Deno can run) in order to align more closely with how the other templates are setup and provide better linting rules and flexibility which is needed for web development, e.g. eslint-plugin-react-hooks gives the rules-of-hooks lint rule that Deno does not have.Furthermore, typechecking is now done with typescript's tsc for all files with
app/
. This allows again for more flexibility by being able to control the typescript version and configuration independent of Deno. In order to provide Deno API access within theapp/
directory (without running into typescript errors), a task calledtypegen
can be run to generate the Deno specific declarations.Closes: denoland/deno#20790