Skip to content

feat(decorators): throw error if called after start#2128

Merged
Eomm merged 1 commit intofastify:masterfrom
dnlup:decorators
Mar 8, 2020
Merged

feat(decorators): throw error if called after start#2128
Eomm merged 1 commit intofastify:masterfrom
dnlup:decorators

Conversation

@dnlup
Copy link
Copy Markdown
Contributor

@dnlup dnlup commented Mar 7, 2020 •
edited
Loading

Check if fastify has started when calling decorate* and throw an error if it has.

Fix #2116 .

Checklist

  • run npm run test and npm run benchmark
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message and code follows Code of conduct

@dnlup
feat(decorators): throw error if called after start
0cceddd 
Check if fastify has started when calling decorate* and
throw an error if it has.

Fix fastify#2116.
@dnlup dnlup changed the title fix(decorators): throw error if called after start feat(decorators): throw error if called after start Mar 7, 2020
mcollina
mcollina approved these changes Mar 7, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

jsumners
jsumners approved these changes Mar 8, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@jsumners jsumners left a comment

Choose a reason for hiding this comment

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

LGTM

Ethan-Arrowood
Ethan-Arrowood approved these changes Mar 8, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@Ethan-Arrowood Ethan-Arrowood left a comment

Choose a reason for hiding this comment

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

LGTM! Nice work

@Eomm Eomm added bugfix Issue or PR that should land as semver patch v2.x Issue or pr related to Fastify v2 labels Mar 8, 2020
@Eomm Eomm merged commit 512561f into fastify:master Mar 8, 2020
@dnlup dnlup deleted the decorators branch March 9, 2020 04:39
@marcbachmann
Copy link
Copy Markdown

marcbachmann commented Mar 23, 2020 •
edited
Loading

Before this PR it was possible to call fastify.decorate even after the server got initialized.

What do you suggest to recover the old behavior? At the moment I could just assign the value directly onto the specific property using fastify.somevalue = value. Or I could mutate the object instead of overriding it multiple times.

edit:... i just saw that I already have some code to mutate the object as it wasn't possible to call the decorate function multiple times. So this was just breaking change that removed support for the delayed .decorate call.

@mcollina
Copy link
Copy Markdown
Member

mcollina commented Mar 23, 2020

It was not really supported before :/. It worked because there were no real checks for it and possibly worked well in your use case.

This was referenced Apr 12, 2020
Merged
Merged
Closed
Merged
This was referenced May 5, 2020
Merged
Merged
Closed
Merged
@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 10, 2022

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@mcollina mcollina mcollina approved these changes

@jsumners jsumners jsumners approved these changes

@Eomm Eomm Eomm approved these changes

+1 more reviewer

@Ethan-Arrowood Ethan-Arrowood Ethan-Arrowood approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bugfix Issue or PR that should land as semver patch v2.x Issue or pr related to Fastify v2

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Make decorate* methods throw after server is started.

6 participants

@dnlup @marcbachmann @mcollina @jsumners @Eomm @Ethan-Arrowood