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

[feat] Prevent js file use from server configuration object #213

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Ahryman40k
Copy link

As discussed in Issue #212, please find here the corresponding pull request.
So, if you agree with adding such feature, you could merge it.

  • Code is fixed and prettify
  • Tests are updated
  • Documentation is updated

Hope I did well according your contributing note and nothing were missed.
Best regards,
Ahry

server configuration allow to set custom metadata. Metadata is loaded
from a path  and we aim to use object instead

Closes bluehalo#212
Update custom metadata documentation file with a new chapter.

Closes bluehalo#212
@Ahryman40k Ahryman40k changed the title [feat] Prevent js file loading from server configuration object [feat] Prevent js file use from server configuration object Nov 29, 2019
@zeevo
Copy link
Collaborator

zeevo commented Dec 2, 2019

Thanks! Will be looking through this.

@zeevo
Copy link
Collaborator

zeevo commented Dec 5, 2019

Hi there. I've been pretty busy lately. However, I'm not sure about the idea of extending the config object to include functions. There is probably a better way, and I will be playing around with TypeScript project that includes node-fhir-server-core to really investigate this. I appreciate your analysis though!

@Ahryman40k
Copy link
Author

I may understand your point of view and of course contrary to the sample I provided, people can provide metadata builder function instead of bare metadata object. It also means that we can dynamically create metadata, a functionality that is missing right now. We can only create them statically at server start.
Moreover it's useful when including you're library into a typescript project not to reference JavaScript files, and it's the main point.

@zeevo
Copy link
Collaborator

zeevo commented Dec 11, 2019

@Ahryman40k Do you have a sample TypeScript project somewhere where I can see this fix in action?

@Ahryman40k
Copy link
Author

Ahryman40k commented Dec 13, 2019

I'm writing a typescript library that encapsulate your own as helper to produce FHIR microservices quickly, but it still under development.
Capabilities route already works and metadata are dynamically added.
I try to share you a sample next week because I'm currently in holiday until next year.
Thank you for your interest.

@Ahryman40k
Copy link
Author

Please find a sample there: https://github.com/Ahryman40k/asymmetrik-sample
A README file is present to run sample.

Don't forget it's a POC and not a real development project.

@zeevo
Copy link
Collaborator

zeevo commented Dec 18, 2019

Thanks @Ahryman40k ! I'll be looking at this to really understand the problem.

@Ahryman40k
Copy link
Author

@zeevosec Have you take a look ?

@zeevo
Copy link
Collaborator

zeevo commented Jan 23, 2020

Hi there @Ahryman40k

I have been super busy with other projects lately. I do understand the issue with dynamically requiring files within TypeScript projects. If I get around to it, we may write some kind of TypeScript compatible Server config mechanism, but until then you may have to just maintain a fork.

Sorry!

@ZuSe
Copy link

ZuSe commented Jul 22, 2020

Would love to see this merged. We are using your great lib in one of our typescript based projects. Would help a lot to maintain a clean code base and setup tests.

@Ahryman40k
Copy link
Author

Me too ;)

@zeevo
Copy link
Collaborator

zeevo commented Aug 30, 2020

Hey I am going to revisit this and probably merge it.

@Ahryman40k
Copy link
Author

Do I need to sync with your repo then resubmit my work ?
I know I did it long time ago.

@luan-dev
Copy link
Member

Hi, I am happy to merge this PR if you can resolve the merge conflicts

@finaldzn
Copy link

Hello any updated for implemeting typescript to the project ?

Thank you

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

5 participants