-
Notifications
You must be signed in to change notification settings - Fork 357
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
Generate dist/dynamic-modules.json for relevant packages #10046
Generate dist/dynamic-modules.json for relevant packages #10046
Conversation
Preview: https://patternfly-react-pr-10046.surge.sh A11y report: https://patternfly-react-pr-10046-a11y.surge.sh |
scripts/parse-dynamic-modules.js
Outdated
|
||
/** @type {ts.CompilerOptions} */ | ||
const defaultCompilerOptions = { | ||
target: ts.ScriptTarget.ES2020, |
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 am wondering if it would be preferred to get some of the compiler options from https://github.com/patternfly/patternfly-react/blob/main/packages/tsconfig.base.json#L9-L12
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.
This code just parses the relevant info from package-specific build outputs, i.e. there is nothing emitted.
I've hardcoded the above TypeScript compiler option defaults to keep things simple and self-contained.
I see no problem with individual scripts defining their own options assuming they represent a separate build step, i.e. my understanding is that packages/tsconfig.base.json
base config is mainly used for building the packages via Rollup.
I can try reusing some options from the base config as suggested, but it shouldn't have any impact on the generated dynamic-modules.json
files.
@vojtechszocs @Hyperkid123
So i'm not sure this is accounting for next components correctly. |
@vojtechszocs I think the next/deprecated modules should not be listed. There are conflicting names and the API difference will cause unexpected issues. |
You are right, module paths containing either "next" or "deprecated" should be excluded. This issue also exists in OpenShift Console dynamic plugin SDK code (dynamic-module-parser.ts). I'll update this PR as well as relevant Console code. |
666d73c
to
6c3bf4f
Compare
@nicolethoen PR updated, please have a look.
|
This PR modifies
scripts/build-single-packages.js
so that adist/dynamic-modules.json
file is generated for each package containing dynamic modules.We generate
dynamic-modules.json
files to make it easier for consuming projects to share specific PatternFly dynamic modules (instead of sharing the whole package indexes) via webpack module federation.Each
dynamic-modules.json
file maps package index exports to their corresponding dynamic modules, for example:The algorithm that parses dynamic module information favors modules with most specific file paths, for example
dist/dynamic/components/Wizard/hooks
is favored overdist/dynamic/components/Wizard
.Dynamic modules nested under
deprecated
ornext
directories are ignored.Note: this code was ported from openshift/console#13521