-
Notifications
You must be signed in to change notification settings - Fork 21
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
refactor(codegen): INFRA-316 update app00 template and add backend miniapp template #4708
base: master
Are you sure you want to change the base?
refactor(codegen): INFRA-316 update app00 template and add backend miniapp template #4708
Conversation
What about template for |
It's out of this pr scope |
packages/codegen/templates/app00/domains/common/components/containers/BaseLayout/BaseLayout.tsx
Outdated
Show resolved
Hide resolved
packages/codegen/templates/app00/domains/common/components/containers/BaseLayout/BaseLayout.tsx
Outdated
Show resolved
Hide resolved
packages/codegen/templates/app00/domains/common/components/containers/BaseLayout/index.ts
Outdated
Show resolved
Hide resolved
} | ||
|
||
module.exports = { | ||
CondoOIDCMiddleware, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may be we need some open-condo/oidc
package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is even a todo by @Alllex202 if Im not mistaken :-D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is even a todo by @Alllex202 if Im not mistaken :-D
I don't remember such a task :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you decide to put this logic in a separate package, then keep in mind that each miniapp may have its own unique logic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you decide to put this logic in a separate package, then keep in mind that each miniapp may have its own unique logic
why would you want to have unique oidc auth logic (especially for our home-cooked miniapps?)
4cfe759
to
f849ef7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add todo to use it in all existing miniapps
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not bin script?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not bin script?
I think it's better to keep the schema generation files in the codegen package, rather than in the root bin, so that the miniaps are not dependent on the root bin. (The truth is now in the package.the json miniap runs a lot of things from the root bin)
packages/codegen/templates/app00/domains/common/constants/index.js
Outdated
Show resolved
Hide resolved
packages/codegen/templates/app01/domains/condo/utils/bots/serviceBot.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about putting this in a separate package?
This really does seem like repetitive logic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that everything about putting the oidc in a separate package is a separate task, this task has already grown so much. Added task and pass TODO here
...ages/codegen/templates/app01/domains/condo/utils/serverSchema/generate.condo.server.utils.js
Outdated
Show resolved
Hide resolved
packages/codegen/templates/app01/domains/condo/utils/serverSchema/index.js
Outdated
Show resolved
Hide resolved
f849ef7
to
01ff192
Compare
|
app00
template – now it's app with keystone and next (hosted by keystone), likepass
miniappapp01
template – it's backend-only app template. In future we need to separate frontend app from backend app for start frontend apps on newnext
versionsNow for start local miniapp development, you need to:
/condo
yarn createapp test-app
. If you want to create keystone + next app, you need to add--type=legacy-keystone-next
flag, if you want to create only keystone app use--type=keystone
flagyarn install
node ./bin/prepare --https --filter=test-app