Skip to content

fix: support npm run dev (remix dev) and Netlify CLI #95

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

Merged
merged 11 commits into from
Mar 2, 2023

Conversation

nickytonline
Copy link
Contributor

@nickytonline nickytonline commented Feb 27, 2023

Description

This fix when the Functions Template is selected, allows a developer to use npm run dev as they would expect, i.e. run remix dev and at the same time allow for use of the Netlify CLI (remix watch).

Related Tickets & Documents

QA Instructions, Screenshots, Recordings

  1. Ensure the Netlify CLI is installed and up to date.
  2. There are two test scenarios:
    • Create a Remix app using TypeScript for Netlify Functions
    • Create a Remix app using JavaScript for Netlify Functions
  3. For each scenario, it always starts by running the following from the root folder in a shell.
npx create-remix --template ./
  1. The CLI will prompt for a location for the project.
❯ npx create-remix --template ./
? Where would you like to create your app? (./my-remix-app)

Ensure that the location is outside of the Netlify Remix template folder, e.g. ../my-remix-app

  1. The Remix CLI will prompt you to choose TypeScript or JavaScript. Choose accordingly for the given test scenarios mentioned above.
? TypeScript or JavaScript? (Use arrow keys)
❯ TypeScript 
  JavaScript
  1. The Remix CLI will prompt you to install packages, press the ENTER to accept the default answer of Yes
? Do you want me to run `npm install`? (Y/n)

The npm packages will install.

  1. Some files will be manipulated if JavaScript is selected. The Remix CLI will output something similar to this.
⠹ Creating your app…⠋ Migrating template to JavaScript…Processing 3 files... 
Spawning 3 workers...
Sending 1 files to free worker...
Sending 1 files to free worker...
Sending 1 files to free worker...
⠴ Migrating template to JavaScript… SKIP /Users/nicktaylor/dev/remix-tests/edgy-ts/globals.d.ts
⠦ Migrating template to JavaScript… OKK /Users/nicktaylor/dev/remix-tests/edgy-ts/remix.init/root.tsx
 OKK /Users/nicktaylor/dev/remix-tests/edgy-ts/remix.init/entry.server.tsx
All done. 
Results: 
0 errors
0 unmodified
1 skipped
2 ok
Time elapsed: 0.428seconds
  1. The Remix CLI extended with our questions will prompt to create the app with Netlify Functions or Netlify Edge Functions.
💿 Running remix.init script
? Run your Remix site with: (Use arrow keys)
❯ Netlify Functions - Choose this for stable support for production sites 
  Netlify Edge Functions (beta) - Try this for improved performance on non-critical sites

10 Select Netlify Functions by pressing the ENTER key.
10. Change to the directory of the project you just created, e.g. ../my-remix-app
13. The netlify.toml for the project should be the same as the one in this PR.
14. Run npm install

Test remix dev

  1. Run
npm run dev
  1. The Remix dev server starts
➜ npm run dev

> dev
> remix dev

Remix App Server started at http://localhost:3000 (http://192.168.86.78:3000)
This is an example of how to set caching headers for a route, feel free to change the value of 60 seconds or remove the header
GET / 200 - - 38.115 ms
This is an example of how to set caching headers for a route, feel free to change the value of 60 seconds or remove the header
GET / 200 - - 2.539 ms
  1. Navigate to http//:localhost:3000 to see the page load.

CleanShot 2023-02-27 at 16 52 03

  1. Make some changes to the text in app/routes/index.tsx (or jsx)
  2. The page reloads with the changes.

Test with Netlify CLI ntl dev

  1. Run ntl dev
  2. The Remix dev server starts up.
  3. Navigate to https://localhost:8888 and the page loads.
  4. Make some changes to the text in app/routes/index.tsx (or jsx)
  5. The page reloads with the changes.

image
For us to review and ship your PR efficiently, please perform the following steps:

Test with Netlify CLI ntl serve

  1. Run npm run start
  2. ntl serve runs (not the Remix dev server)
  3. The _custom_redirects file gets copied to public/_redirects
  4. Navigate to https://localhost:8888 and the page loads.

image
For us to review and ship your PR efficiently, please perform the following steps:

  • Open a bug/issue before writing your code 🧑‍💻. This ensures we
    can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or
    something that`s on fire 🔥 (e.g. incident related), you can skip this step.
  • Read the contribution guidelines 📖. This ensures your code follows our style
    guide and passes our tests.
  • Update or add tests (if any source code was changed or added) 🧪
  • Update or add documentation (if features were changed or added) 📝
  • Make sure the status checks below are successful ✅

A picture of a cute animal (not mandatory, but encouraged)

A hamster washing their paws in a miniature sink

@netlify
Copy link

netlify bot commented Feb 27, 2023

Deploy Preview for remix-edge-on ready!

Name Link
🔨 Latest commit d977e56
🔍 Latest deploy log https://app.netlify.com/sites/remix-edge-on/deploys/6400f7be01eeba0008166be5
😎 Deploy Preview https://deploy-preview-95--remix-edge-on.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 site settings.

@github-actions github-actions bot added the type: bug code to address defects in shipped code label Feb 27, 2023
@@ -2,6 +2,7 @@ node_modules

/.cache
/public/build
/build
Copy link
Contributor Author

Choose a reason for hiding this comment

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

remix dev generates these artifacts as well as those in /public/build

@nickytonline nickytonline self-assigned this Feb 27, 2023
@nickytonline nickytonline marked this pull request as ready for review February 27, 2023 21:58
@nickytonline nickytonline requested a review from a team February 27, 2023 21:58
@nickytonline nickytonline changed the title fix: now we support npm run dev (remix dev) and netlify CLI fix: support npm run dev (remix dev) and netlify CLI Feb 27, 2023
@nickytonline nickytonline changed the title fix: support npm run dev (remix dev) and netlify CLI fix: support npm run dev (remix dev) and Netlify CLI Feb 27, 2023
@ascorbic
Copy link
Contributor

The behaviour for all other frameworks is for netlify dev to run the framework's dev server. The only reason Remix is different is because when we first added it, there was no remix dev command. I think we should do the same here: use the dev server for netlify dev. If people want to try a production build, they can use netlify serve.

@nickytonline
Copy link
Contributor Author

nickytonline commented Feb 28, 2023

The behaviour for all other frameworks is for netlify dev to run the framework's dev server. The only reason Remix is different is because when we first added it, there was no remix dev command. I think we should do the same here: use the dev server for netlify dev. If people want to try a production build, they can use netlify serve.

The problem with using remix dev is it does not support custom server configurations which is why we went with remix watch, i.e. serverBuildTarget: "netlify", in remix.config.js. Other server build targets have the same issue as far as I know. Actually looking at some other providers, we can pretty much omit the serverBuildTarget as all we're doing with it is setting this which I can set in the remix.config.js

serverBuildPath = ".netlify/functions-internal/server.js";

I'll try that out.

@@ -4,7 +4,7 @@
"scripts": {
"build": "remix build",
"dev": "remix dev",
"start": "cross-env NODE_ENV=production netlify dev",
"start": "netlify serve",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using ntl serve as it makes more sense in this context.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd normally expect start to run dev, but I guess it could go either way. Adding a serve command could be good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was going with your suggestion from the other day unless I was dreaming. 🙃 . Using serve avoids running build explicitly which is nice and ntl dev still gives us dev mode

Copy link
Contributor

Choose a reason for hiding this comment

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

What I meant was that if people wanted the previous behaviour (i.e. running the prod server) then they could use ntl serve instead

@@ -1,9 +1,9 @@
const baseConfig =
process.env.NETLIFY || process.env.NETLIFY_LOCAL
process.env.NODE_ENV === "production"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We'll only pull in the custom server settings if building for production, i.e. ntl serve, ntl build,ntl deploy or via git push.

@nickytonline
Copy link
Contributor Author

Giving you a tag @whitep4nth3r as we'll be syncing up on this Thursday.

@nickytonline nickytonline force-pushed the nickytonline/fixes-to-netlify-functions-template branch from 8b6cb08 to c5b842d Compare March 2, 2023 16:30
@@ -0,0 +1,10 @@
# This file is being used instead of the typical Netlify _redirects file in the root of your application
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I renamed it from _custom_redirects to _app_redirects @whitep4nth3r. I think it conveys what the file is better along with the comments in the file. Thoughts on the wording? cc: @stephmarie17

Choose a reason for hiding this comment

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

Much better I think!

sarahetter
sarahetter previously approved these changes Mar 2, 2023
Copy link

@sarahetter sarahetter left a comment

Choose a reason for hiding this comment

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

Tested both js and ts with netlify functions, and both work as described 🎉

Copy link
Contributor

@stephmarie17 stephmarie17 left a comment

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Netlify Functions Template Errors when running netlify dev
5 participants