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.

Sign up for GitHub

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

Jump to bottom

Throw an error if dev scripts contain now dev in package.json #2507

Merged
merged 4 commits into from
Jul 9, 2019
Merged

Throw an error if dev scripts contain now dev in package.json #2507

merged 4 commits into from
Jul 9, 2019

Conversation

thasophearak
Copy link
Contributor

This PR throw an error message if dev scripts contain now dev in user package.json.

@thasophearak thasophearak requested a review from leo July 8, 2019 13:32
@codecov-io
Copy link

codecov-io commented Jul 8, 2019 •
edited

Codecov Report

Merging #2507 into canary will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           canary    #2507      +/-   ##
==========================================
- Coverage   11.94%   11.93%   -0.01%     
==========================================
  Files         257      257              
  Lines        9304     9315      +11     
  Branches     1046     1049       +3     
==========================================
+ Hits         1111     1112       +1     
- Misses       8072     8082      +10     
  Partials      121      121
Impacted Files Coverage Δ
src/util/dev/types.ts 0% <ø> (ø) ⬆️
src/commands/dev/index.ts 0% <0%> (ø) ⬆️
src/util/metrics.ts 61.53% <0%> (-6.65%) ⬇️

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 9919701...8987abe. Read the comment docs.

@thasophearak thasophearak marked this pull request as ready for review July 8, 2019 13:39
styfle
styfle reviewed Jul 8, 2019
View reviewed changes
src/commands/dev/index.ts Outdated Show resolved Hide resolved
@leo leo removed the request for review from TooTallNate July 8, 2019 14:48
@leo leo requested a review from AndyBitz July 8, 2019 14:48
AndyBitz
AndyBitz requested changes Jul 8, 2019
View reviewed changes
src/commands/dev/index.ts Outdated Show resolved Hide resolved
@TooTallNate
Copy link
Member

@Sophearak Is this because @now/static-build now respects dev script or now-dev script?

@thasophearak
Copy link
Contributor Author

@TooTallNate Yes, exactly. Since zero-config is expected dev as a command to build static-site or app instead of now dev.

leo
leo suggested changes Jul 9, 2019
View reviewed changes
src/commands/dev/index.ts Outdated Show resolved Hide resolved
@thasophearak thasophearak merged commit e0489dc into canary Jul 9, 2019
@thasophearak thasophearak deleted the update/now-dev-package branch July 9, 2019 10:11
AndyBitz pushed a commit that referenced this pull request Jul 12, 2019
@thasophearak @AndyBitz
Throw an error if dev scripts contain now dev in package.json (#2507
e57cf65 
)

* Throw an error if `dev` scripts contain `now dev` in package.json

* Use proper Package type

* Update src/commands/dev/index.ts
@Pushplaybang
Copy link

can i ask what the intention is for the rest of who have configured our projects?

@Pushplaybang Pushplaybang mentioned this pull request Jul 13, 2019
Closed
@juanpernu
Copy link

Hi guys! How are you? Sorry for writing here but it would be great if you tell us how the script must be in the package-json because this fix is really working 😛 Thanks!
image

@styfle
Copy link
Member

styfle commented Jul 13, 2019 •
edited

@Pushplaybang When now dev was introduced, it needed a separate now-dev script to detect a server such as gatsby https://zeit.co/blog/now-dev

This has since been changed so that the dev script can be used. This PR is necessary to prevent an infinite loop where now dev invokes now dev.

@AndyBitz AndyBitz mentioned this pull request Jul 16, 2019
Closed
@zachlewis2020 zachlewis2020 mentioned this pull request May 13, 2024
Open
@svcsnyksanity svcsnyksanity mentioned this pull request May 13, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TooTallNate TooTallNate TooTallNate left review comments

@styfle styfle styfle left review comments

@leo leo leo approved these changes

@AndyBitz AndyBitz AndyBitz approved these changes

Assignees

@AndyBitz AndyBitz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@thasophearak @codecov-io @TooTallNate @Pushplaybang @juanpernu @styfle @leo @AndyBitz