Skip to content

[Typescript][Angular] Added in injected basePath observable to api service to allow it to respond to external updates to this setting post construction.#7965

Closed
chrismellard wants to merge 7 commits into
swagger-api:masterfrom
DatacomNelson:issue-7949
Closed

[Typescript][Angular] Added in injected basePath observable to api service to allow it to respond to external updates to this setting post construction.#7965
chrismellard wants to merge 7 commits into
swagger-api:masterfrom
DatacomNelson:issue-7949

Conversation

@chrismellard

@chrismellard chrismellard commented Apr 4, 2018

Copy link
Copy Markdown

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

Fixes #7949

Extended api service to allow optional injection of an observable basePath. This facilitates configuration of basePath post service construction which is required in circumstances where runtime setting of basePath cannot be achieved using APP_INITIALIZER tokens in angular. One such use case is when using libraries such as ngrx effects that do not respect such angular APP_INITIALIZER tokens.

Another option is to simply include the injected observable as part of the Configuration class rather than a separate dependency injectable token. If its preferable to move it inside that class I can do so.

…espond to external updates to this setting post construction.

Added in injected basePath observable to api service to allow it to respond to external updates to this setting post construction.
@chrismellard

Copy link
Copy Markdown
Author

@macjohnny

@macjohnny macjohnny left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks good to me

@macjohnny

Copy link
Copy Markdown
Contributor

@chrismellard thanks for the PR. would you mind adding a note to the Readme.moustache or add a description of the constructor variable?

@macjohnny

Copy link
Copy Markdown
Contributor

cc @TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01)

@sebastianhaas

Copy link
Copy Markdown
Contributor

Will check tomorrow!

@chrismellard

Copy link
Copy Markdown
Author

@macjohnny Sure. Just pushed an update to the README.mustache file with some example usage.

@macjohnny

Copy link
Copy Markdown
Contributor

@wing328 could you please restart the CircleCI tests?
@chrismellard could you please merge the current master into your branch and re-generate the samples?

@TiFu

TiFu commented Apr 19, 2018 via email

Copy link
Copy Markdown
Contributor

@macjohnny macjohnny left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Use the ‚{#useRxJs6}‘ variable

@chrismellard

Copy link
Copy Markdown
Author

Thanks - will update now

@digaus

digaus commented Nov 20, 2019

Copy link
Copy Markdown

Any reason why this is not merged yet?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[typescript-angular] service configuration/basePath modification after construction

6 participants