Skip to content
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

[now dev] Use the dev server cwd as builder's workPath #2326

Merged
merged 4 commits into from
May 11, 2019

Conversation

TooTallNate
Copy link
Member

@TooTallNate TooTallNate commented May 8, 2019

Rather than copying the source files into a temporary directory, simply use the existing source files in the cwd. This will make subsequent boots of now dev be faster (i.e. because the node_modules directory will already be in place), as well as use much less space on the filesystem because temporary directory are no longer being used.

This will require some changes to the builders and @now/build-utils, to ensure that the download() function is always installing into workPath, and that the meta object passed to the build() function is also passed into the download() function.

For example:

Blockers:

  • Since the builders will be installing stuff to the same directory as the cwd dir being watched by nsfw (i.e. node_modules), the watcher will error on some platforms (for example Windows) because there's too many files. nsfw currently does not have "ignore" capabilities, see How to ignore certain subdirectories? Axosoft/nsfw#54.

@codecov-io
Copy link

codecov-io commented May 8, 2019

Codecov Report

Merging #2326 into canary will increase coverage by 0.89%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           canary    #2326      +/-   ##
==========================================
+ Coverage   10.63%   11.52%   +0.89%     
==========================================
  Files         247      247              
  Lines        9052     9047       -5     
  Branches      994      994              
==========================================
+ Hits          963     1043      +80     
+ Misses       7979     7894      -85     
  Partials      110      110
Impacted Files Coverage Δ
src/commands/dev/lib/dev-server.ts 53.46% <ø> (+12.3%) ⬆️
src/commands/dev/lib/types.ts 0% <ø> (ø) ⬆️
src/commands/dev/lib/dev-builder.ts 69.82% <50%> (+14.07%) ⬆️
src/util/get-files.ts 89.61% <0%> (+1.29%) ⬆️
src/commands/dev/lib/dev-router.ts 75.47% <0%> (+1.88%) ⬆️
src/util/output/create-output.ts 37.83% <0%> (+2.7%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39591b4...bb38072. Read the comment docs.

@leo
Copy link
Contributor

leo commented May 8, 2019

Can we add a basic integration test that runs now dev with @now/next and makes a request to the server?

Rather than copying the source files into a temporary directory,
simply use the existing source files in the `cwd`. This will make
subsequent boots of `now dev` be faster (i.e. because the `node_modules`
directory will already be in place), as well as use much less space on
the filesystem because temporary directory are no longer being used.

This will require some changes to the builders and `@now/build-utils`,
to ensure that the `download()` function is always installing into
`workPath`, and that the `meta` object passed to the `build()` function
is also passed into the `download()` function.

For example:

 - vercel/now-builders#474
 - vercel/now-builders#475
This makes `download()` be a no-op when running in `now dev`.
@TooTallNate TooTallNate force-pushed the update/now-dev-use-cwd-as-workPath branch from e2bc5cb to a66e046 Compare May 10, 2019 18:12
@TooTallNate TooTallNate changed the title [now dev] WIP Use the dev server cwd as builder's workPath [now dev] Use the dev server cwd as builder's workPath May 10, 2019
@TooTallNate TooTallNate merged commit 385c4e4 into canary May 11, 2019
@TooTallNate TooTallNate deleted the update/now-dev-use-cwd-as-workPath branch May 11, 2019 00:13
leo pushed a commit that referenced this pull request May 20, 2019
* [now dev] Use the dev server `cwd` as builder's `workPath`

Rather than copying the source files into a temporary directory,
simply use the existing source files in the `cwd`. This will make
subsequent boots of `now dev` be faster (i.e. because the `node_modules`
directory will already be in place), as well as use much less space on
the filesystem because temporary directory are no longer being used.

This will require some changes to the builders and `@now/build-utils`,
to ensure that the `download()` function is always installing into
`workPath`, and that the `meta` object passed to the `build()` function
is also passed into the `download()` function.

For example:

 - https://github.com/zeit/now-builders/pull/474
 - https://github.com/zeit/now-builders/pull/475

* Update `@now/build-utils` to v0.5.5-canary.1

This makes `download()` be a no-op when running in `now dev`.

* Add test case for `@now/next`

* Fix debug log formatting
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.

None yet

3 participants