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

Warning for custom build scripts in project's package.json #5240

Closed
wants to merge 11 commits into from
Closed

Warning for custom build scripts in project's package.json #5240

wants to merge 11 commits into from

Conversation

chalosalvador
Copy link
Member

@chalosalvador chalosalvador commented Nov 14, 2022 •
edited

Description

Warn if the build command in the project package.json contains anything other than the framework's default build command. Mention that we don’t call that command & they should [use custom (https://firebase.google.com/docs/hosting/express)

Scenarios Tested

Tested that the warning is shown when the build script has anything other than:

  • Next -> next build
  • Angular -> ng build
  • Nuxt not in the list of frameworks when firebase init hosting
  • Lit -> vite build or tsc && vite build
  • React -> vite build or tsc && vite build
  • Svelte -> vite build
  • Preact -> vite build or tsc && vite build
  • Vite -> vite build or tsc && vite build

@chalosalvador
Add warning for custom build scripts
6514468
@chalosalvador
Allow TS build commands
884f810 
- Reuse message and allow injecting commands for each framework.
@chalosalvador chalosalvador marked this pull request as ready for review November 18, 2022 14:11
@jamesdaniels jamesdaniels self-requested a review November 23, 2022 20:55
@jamesdaniels jamesdaniels self-assigned this Nov 23, 2022
jamesdaniels
jamesdaniels requested changes Dec 1, 2022
View reviewed changes
Copy link
Member

@jamesdaniels jamesdaniels left a comment

Choose a reason for hiding this comment

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

Add changelog entry

src/frameworks/angular/index.ts Outdated Show resolved Hide resolved
src/frameworks/utils.ts Show resolved Hide resolved
src/frameworks/nuxt/index.ts Outdated Show resolved Hide resolved
jamesdaniels
jamesdaniels requested changes Dec 2, 2022
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
src/frameworks/utils.ts Show resolved Hide resolved
@jamesdaniels jamesdaniels mentioned this pull request Dec 8, 2022
Merged
@jamesdaniels
Copy link
Member

@chalosalvador can you address the prettier concern with the changelog?

@chalosalvador
Copy link
Member Author

@jamesdaniels done. Prettier should be happy now.

jamesdaniels added a commit that referenced this pull request Dec 9, 2022
@jamesdaniels
Warning for custom build scripts in project's package.json (#5240)
d7f0186 
Warn if the build command in the source directory's package.json contains anything other than the default build command. Direct users towards the Express.js / custom integration
@jamesdaniels
Copy link
Member

Merged in #5315, thanks @chalosalvador

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jamesdaniels jamesdaniels jamesdaniels approved these changes

Assignees

@jamesdaniels jamesdaniels

Labels
integration: web frameworks
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@chalosalvador @jamesdaniels