feat: add `--prod` flag to `dev` command #5338

eduardoboucas · 2022-12-23T13:03:48Z

Summary

github-actions · 2022-12-23T13:05:22Z

📊 Benchmark results

Comparing with ab48f8e

Package size: 259 MB

⬆️ 0.03% increase vs. ab48f8e

^                          259 MB  259 MB  259 MB  259 MB  259 MB  259 MB  259 MB  259 MB  259 MB  259 MB 
│  249 MB  249 MB  242 MB   ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐  
│   ┌──┐    ┌──┐    ┌──┐    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-12    T-11    T-10    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T

Legend

T-30 (f9f45ed): 249 MB
T-29 (22de9e2): 249 MB
T-28 (d64c875): 249 MB
T-27 (cd993b0): 249 MB
T-26 (9a7c666): 249 MB
T-25 (cf2f402): 249 MB
T-24 (6102e77): 249 MB
T-23 (d3dfa17): 249 MB
T-22 (9bb51a1): 249 MB
T-21 (81ac0c6): 249 MB
T-20 (bb16cec): 249 MB
T-19 (3ff4a57): 249 MB
T-18 (3b491be): 249 MB
T-17 (0bc18bb): 249 MB
T-16 (12664f3): 249 MB
T-15 (7ebf238): 249 MB
T-14 (df9260a): 249 MB
T-13 (e3db314): 249 MB
T-12 (7eb4479): 249 MB
T-11 (89b14e2): 249 MB
T-10 (e1f661a): 242 MB
T-9 (2994d3e): 259 MB
T-8 (aa3939a): 259 MB
T-7 (61675b2): 259 MB
T-6 (2f1f8fd): 259 MB
T-5 (3f3f9c9): 259 MB
T-4 (af19b68): 259 MB
T-3 (21d9191): 259 MB
T-2 (16376e9): 259 MB
T-1 (ab48f8e): 259 MB
T (current commit): 259 MB

eduardoboucas · 2023-01-02T12:33:08Z

src/commands/dev/dev.mjs

@@ -710,6 +807,7 @@ export const createDevCommand = (program) => {
    .option('-o ,--offline', 'disables any features that require network access')
    .option('-l, --live', 'start a public live session', false)
    .option('--functionsPort <port>', 'port of functions server', (value) => Number.parseInt(value))
+    .option('-s, --serve', 'run in "serve" mode', false)


This needs a better description.

How about --prod instead? We're running it in a prod-like mode

Yeah, I like that!

@ticachica any thoughts/concerns here?

Renamed to --prod in 78c35b6.

eduardoboucas · 2023-01-02T12:35:12Z

src/lib/functions/registry.mjs

@@ -145,11 +148,12 @@ export class FunctionsRegistry {
      )
    }

-    // This fixes the bug described here https://github.com/netlify/zip-it-and-ship-it/issues/637
-    // If the current function's file is a zip bundle, we ignore it and log a helpful message.
+    // If the function file is a ZIP, we extract it and rewire its main file to


We need to do this so that the functions have all its dependencies available, otherwise we'd need to load modules from .netlify/plugins/node_modules, which isn't part of the module resolution path. We know we've already bundled the functions as part of the build process, so we can extract their ZIP files and use the exact files that will be used in Lambda, in the spirit of simulation the production environment as closely as possible.

src/commands/dev/dev.mjs

ericapisani · 2023-01-03T13:20:47Z

src/commands/dev/dev.mjs

+
+  // When using the `serve` flag, we want to use the production functions built
+  // by Netlify Build rather than building them from source.
+  const dist = Boolean(options.serve)


Is it possible to change the name of this variable to something more along of the lines of what the comment above is suggesting? I worry that since dist is often used as a name for where built files are output to during a site build that this will be confusing for a future dev.

Totally. Done in d782283.

docs/commands/dev.md

src/commands/dev/dev.mjs

ascorbic · 2023-01-03T17:13:31Z

src/commands/dev/dev.mjs

+      command: undefined,
+      useStaticServer: true,
+    })
+


Suggested change

log(`${NETLIFYDEVLOG} Serving production build \n${NETLIFYDEVWARN} Changed files will not be rebuilt until you restart the server`)

src/commands/dev/dev.mjs

ascorbic · 2023-01-03T17:24:51Z

Also needs changing for edge functions: https://github.com/netlify/cli/blob/feat/serve-command/src/lib/edge-functions/registry.mjs#L223

Co-authored-by: Matt Kane <m@mk.gg>

eduardoboucas · 2023-01-03T17:47:19Z

Also needs changing for edge functions: https://github.com/netlify/cli/blob/feat/serve-command/src/lib/edge-functions/registry.mjs#L223

I'd rather do that in a follow-up PR. I don't like the idea of making the edge functions registry aware of the prod flag, so I want to shift things around a bit, but that would make this PR too large.

eduardoboucas · 2023-01-03T18:01:50Z

Actually, I managed to do it in a concise way in 2934233.

codebyuma · 2023-01-03T19:37:50Z

docs/commands/dev.md

@@ -30,6 +30,7 @@ netlify dev
 - `live` (*boolean*) - start a public live session
 - `offline` (*boolean*) - disables any features that require network access
 - `port` (*string*) - port of netlify dev
+- `prod` (*boolean*) - build the site for production and serve locally


Just creeping on your PR 😄 How does this work with or in contrast to the context flag (documented above), that you can also set to production ?

ie netlify dev --context production

(the CLI doc above mentions only environment variables, but our docs site makes it sound like using the context flag builds your site for production - so this would need to be updated if that's not correct)

Your creeping is very much welcome and that's a great point! The CLI docs are correct: the context flag only affects environment variables. This makes me wonder whether we should repurpose context and introduce this new behaviour (i.e. actually building the site for production) when context is production. And we could make --prod an alias for --context=production.

I think that would make things more consistency, with just two concerns worth considering:

That would make this a breaking change, since we'd be changing an existing flag rather than introducing a new one

This new behaviour means that watching for files won't work, and people will have to restart the CLI for the build to be run again and their changes to be picked up. I worry that it might be confusing that a specific value of context triggers this behaviour, rather than an entirely new flag.

But if we decide not to reuse context, it'll be a bit difficult to explain why you have --prod and --context=production.

@ascorbic What are your thoughts on this one?

I thought about this a bit more and I feel that a more manageable step would be to keep context as is (i.e. just about environment variables) and make prod automatically set --context=production, so that you're building the production site and using the production env vars.

Now that we have named the flag prod, I agree that making it automatically set --context=production feels most intuitive

This is why I sugegsted having it as a separate command. Using ntl dev when it's building for production is a little unintuitive

Building a separate command is a bigger lift (if done properly) that unfortunately I don't have the capacity for right now. I've pushed ae74853 which sets --context=production when --prod is set, and in f3cc600 added a "beta" label to the flag in the help menu. We do this for other commands.

I suggest that we ship this as is and then iterate on it. If we come to the conclusion that this makes more sense as a separate command, we can break it out and deprecate --prod.

Yeah, that is a good compromise. Does our CLI lib support aliases? If so, that would probably be the simplest solution.

#5351 turns the flag into a separate command. We can get it merged after this one.

eduardoboucas · 2023-01-04T11:24:46Z

src/commands/dev/dev.mjs

@@ -713,7 +822,6 @@ export const createDevCommand = (program) => {
      '--context <context>',
      'Specify a deploy context or branch for environment variables (contexts: "production", "deploy-preview", "branch-deploy", "dev")',
      normalizeContext,
-      'dev',


By setting a default value here, we don't give commands the opportunity to set their own custom values. I've removed the default and added it in https://github.com/netlify/cli/pull/5338/files#diff-67082469659283bc342bd13f7e20570cb3d1bf5096aad88545a9ac4809d6083bR520.

eduardoboucas · 2023-01-04T12:38:38Z

src/commands/base-command.mjs

+   */
+  getDefaultContext() {
+    const { prod } = this.opts()
+    const isDevCommand = ['dev', 'dev:exec'].includes(this.name())


Really not a fan of having command-specific logic in the base command, but by the time the command runs it's too late to change the context because the base command will have already retrieved the config. We should revisit this and build some sort of hook that commands can set.

ascorbic · 2023-01-06T16:17:58Z

src/commands/dev/dev.mjs

+    }
+
+    // Run Netlify Build using the main entry point.
+    await buildSite(buildSiteOptions)


If the build fails then it doesn't cause the server to exit. This is fine for normal dev mode, where you can fix the code and it will then recompile, but with --prod there's no hot-reloading, so it sticks with the server running but no built site. If there's a build failure in prod it shoudl cause the server to exit

Agreed. I've done it in f4f31fb to avoid merge conflicts.

Once you're happy with this PR, I can merge that one in.

eduardoboucas · 2023-01-09T14:02:27Z

src/commands/dev/dev.mjs

+    // Add `NETLIFY_DEV` to the environment variables.
+    env.NETLIFY_DEV = { sources: ['internal'], value: 'true' }
+
+    // Ensure the internal functions directory exists so that any functions


Will move this to a more suitable place in #5351.

ascorbic

🎉

feat: add --serve flag to dev command

087f7dd

eduardoboucas added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Dec 23, 2022

eduardoboucas added 4 commits December 23, 2022 15:22

refactor: simplify internal directory clean up

b3f582d

refactor: load built functions

90402fd

refactor: make things nicer

d5652e7

refactor: add const

ac6a845

eduardoboucas commented Jan 2, 2023

View reviewed changes

feat: show debug message

134bf3f

ascorbic reviewed Jan 3, 2023

View reviewed changes

src/commands/dev/dev.mjs Outdated Show resolved Hide resolved

ericapisani reviewed Jan 3, 2023

View reviewed changes

eduardoboucas added 8 commits January 3, 2023 13:35

refactor: rename variable

d782283

Merge branch 'main' into feat/serve-command

7a0d578

chore: add extract-zip dependency

5411c3c

chore: update snapshots

05e9dc3

refactor: conditionally set quiet flag

3ec828b

refactor: simplify code

1a89050

chore: update docs

2d8d2d0

refactor: rename flag to prod

78c35b6

eduardoboucas changed the title ~~feat: add --serve flag to dev command~~ feat: add --prod flag to dev command Jan 3, 2023

refactor: update message

0bcc9a2

eduardoboucas marked this pull request as ready for review January 3, 2023 16:04

eduardoboucas requested a review from a team January 3, 2023 16:04

eduardoboucas requested a review from a team as a code owner January 3, 2023 16:04

eduardoboucas added 2 commits January 3, 2023 17:07

feat: do not set NETLIFY_DEV when prod is set

4090013

chore: remove lint exceptions

9be736b

ascorbic reviewed Jan 3, 2023

View reviewed changes

Apply suggestions from code review

e3eea4a

Co-authored-by: Matt Kane <m@mk.gg>

refactor: treat NETLIFY_DEV consistently across function types

2934233

eduardoboucas added 4 commits January 3, 2023 18:23

refactor: add internal source for env vars

ebb2334

fix: handle internal vars correctly

01df8a3

chore: update docs

59297c1

refactor: remove unused variable

917fc13

codebyuma reviewed Jan 3, 2023

View reviewed changes

eduardoboucas added 2 commits January 4, 2023 11:18

feat: set context=production when prod is used

ae74853

refactor: add beta label

f3cc600

eduardoboucas commented Jan 4, 2023

View reviewed changes

eduardoboucas mentioned this pull request Jan 6, 2023

feat: add serve command #5351

Merged

ascorbic reviewed Jan 6, 2023

View reviewed changes

eduardoboucas added 6 commits January 9, 2023 12:30

Merge branch 'main' into feat/serve-command

f4a2dfd

chore: remove ESLint directive

dbd9dcc

fix: fix imports

bc5c59d

refactor: always start functions server

496783a

refactor: ensure internal functions directory

a92f755

refactor: reinstate functions server check

af00758

eduardoboucas commented Jan 9, 2023

View reviewed changes

eduardoboucas requested review from ascorbic and orinokai January 9, 2023 14:41

ascorbic approved these changes Jan 9, 2023

View reviewed changes

eduardoboucas merged commit 9023210 into main Jan 9, 2023

eduardoboucas deleted the feat/serve-command branch January 9, 2023 14:57

token-generator-app bot mentioned this pull request Jan 9, 2023

chore(main): release 12.7.0 #5356

Merged

eduardoboucas mentioned this pull request Jan 24, 2023

fix: load internal environment variables in Envelope #5429

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add `--prod` flag to `dev` command #5338

feat: add `--prod` flag to `dev` command #5338

eduardoboucas commented Dec 23, 2022

github-actions bot commented Dec 23, 2022 •

edited

Loading

eduardoboucas Jan 2, 2023

ascorbic Jan 3, 2023

eduardoboucas Jan 3, 2023

eduardoboucas Jan 3, 2023

eduardoboucas Jan 3, 2023

eduardoboucas Jan 2, 2023

ericapisani Jan 3, 2023

eduardoboucas Jan 3, 2023

ascorbic Jan 3, 2023

ascorbic commented Jan 3, 2023 •

edited

Loading

eduardoboucas commented Jan 3, 2023

eduardoboucas commented Jan 3, 2023

codebyuma Jan 3, 2023

codebyuma Jan 3, 2023

eduardoboucas Jan 3, 2023

eduardoboucas Jan 3, 2023

orinokai Jan 4, 2023

ascorbic Jan 4, 2023

eduardoboucas Jan 4, 2023

ascorbic Jan 4, 2023

eduardoboucas Jan 6, 2023

eduardoboucas Jan 4, 2023

eduardoboucas Jan 4, 2023

ascorbic Jan 6, 2023

eduardoboucas Jan 6, 2023

eduardoboucas Jan 9, 2023

ascorbic left a comment


	log(`${NETLIFYDEVLOG} Serving production build \n${NETLIFYDEVWARN} Changed files will not be rebuilt until you restart the server`)

feat: add --prod flag to dev command #5338

feat: add --prod flag to dev command #5338

Conversation

eduardoboucas commented Dec 23, 2022

Summary

github-actions bot commented Dec 23, 2022 • edited Loading

📊 Benchmark results

Package size: 259 MB

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ascorbic commented Jan 3, 2023 • edited Loading

eduardoboucas commented Jan 3, 2023

eduardoboucas commented Jan 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ascorbic left a comment

Choose a reason for hiding this comment

feat: add `--prod` flag to `dev` command #5338

feat: add `--prod` flag to `dev` command #5338

github-actions bot commented Dec 23, 2022 •

edited

Loading

ascorbic commented Jan 3, 2023 •

edited

Loading