-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Make shared component build work in isolation #31066
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
base: develop
Are you sure you want to change the base?
Conversation
* Add deps that were missing because they were getting picked up from element-web main but shared-components needs itself * Exclude test files from dts generation * Bump version
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.
Can we take this opportunity to tidy the deps, e.g. temporal-polyfill is now depended on by both shared-components and element-web but actually only used by shared-components... Or is this needed because we bypass the dependencies of the subpackage and its build system? in which case that feels super brittle and we should stop doing that otherwise we might end up with shared components in isolation working but within element web not due to dep version mismatches
|
Indeed, we do need the dependencies as we import the typescript source directly, like we do with js-sdk, although that has fewer dependencies so I assume it's never been an issue. We can certainly switch to consuming the built output of shared-components, it will mean more steps to run for dev but is probably the "right" choice - wdyt? |
We install js-sdk as a dependency so we get its dependencies transitively, we don't git clone it and reference it by fs path. We need certain additional dev deps because we use the src, rather than lib but no runtime deps.
I think it is the right choice, we could use |
|
I suppose if we just added it as a dependency then we'd still get the dependencies transitively and we could continue importing the ts directly? |
|
Yes that is an option, there is always potential for inconsistency between ew and other consumers given the build system is being bypassed |
scripts/docker-package.sh
Outdated
| DIST_VERSION=$("$DIR"/normalize-version.sh "$DIST_VERSION") | ||
|
|
||
| # Build resources as the shared components need them | ||
| yarn run build:res |
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.
should this maybe be in shared-component prepare 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.
yeah, possibly - trying it
* build:res on the parent as part of shared component prepare * link shared component repo inn docker build
so it gets bundled rather than left as a relative import to the json file
because we're patching a dev dependency so it won't be there if we're installed as a dependency
Checklist
public/exportedsymbols have accurate TSDoc documentation.