-
Notifications
You must be signed in to change notification settings - Fork 724
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
docs: clarify emitDecoratorMetadata requirement #1693
base: master
Are you sure you want to change the base?
docs: clarify emitDecoratorMetadata requirement #1693
Conversation
emitDecoratorMetadata is not available in some environments so it is helpful to clarify that it is not strictly required. Also I broke out a separate sub-section for TypeScript under Installation.
Hey @camsteffen, thanks for your contribution. You're absolutely right. Having said that, these docs are legacy and are about to be replaced. Would you consider having a look at the new docs? I would prefer to update the new docs in the monorepo. I think we could explain the nuances of automatic class injection in the Your feedback would be more than welcome in this discussion. |
Thanks for taking a look @notaphplover. The new docs looks promising!
Sounds good to me. How about an I think it's also helpful to have an overview of the tsconfig options under Getting Started. It looks like the blurb that I edited in this PR could be very similarly applied to the new Getting Started page. I could open a PR with that change if you like. Another thing I'd like to see in the docs is a clear explanation of what I was initially confused by docs/ vs. packages/docs/ in the monorepo. Maybe rename docs/ to contributing/. |
Hey @camsteffen
Sounds great!
As long as
Yeah, it's established in the API docs, but I honestly believe the Injection page could have a reference to this page so it's easier to find it out.
Yeah, those are the developer docs. I would prefer to keep the directory as it is, but I don't mind clarifying in the introduction page where the user docs are placed. |
Even this part?
Hmm, to be honest that section isn't very helpful to me. I would want "When is injectable mandatory?" to tell me what InversifyJS features are enabled by using the decorator. I think it's pretty common for documentation to have a Guide section and an API section. So honestly when I see Fundamentals and API, I think "oh Fundamentals must be Guide". |
Hey @camsteffen
Ohh, no, I mean keeping developer docs directory. All right, I am open to try another approach, I would love to see your proposal in the PR. As long as we cover the concepts already covered I don't mind changing the fundamentals section. To be honest I've been struggling to make docs readable and easy to understand. As an english as a second language speaker, I do appreciate the feedback of a native speaker to polish docs. As a maintained who have a full understanding of the library, almost everything seems readable and easy to understand, so I'm more than glad to reive this feedback. You're more than welcome to make your proposal If you need any sort of clarification we can discuss it in the PR or even schedule a meeting |
Description
emitDecoratorMetadata is not available in some environments so it is helpful to clarify that it is not strictly required. Also I broke out a separate sub-section for TypeScript under Installation.
Related Issue
Related: #1493
Motivation and Context
I wanted to transition to using tsx runtime which does not support
emitDecoratorMetadata
.How Has This Been Tested?
Types of changes
Checklist: