-
Notifications
You must be signed in to change notification settings - Fork 156
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
[devops]: Remove references to internal/vendor packages #12010
[devops]: Remove references to internal/vendor packages #12010
Conversation
Deploy preview created for package Built with commit: e47e8c17a1faef713e3a51384ded928293744149 |
Deploy preview created for package Built with commit: 5275b71534400d9f7867c9fe4682278c7dadf334 |
Deploy preview created for package Built with commit: 299e7d8758e02815422a6fd13d6880a37d1f0ae6 |
Deploy preview created for package Built with commit: 299e7d8758e02815422a6fd13d6880a37d1f0ae6 |
Deploy preview created for package Built with commit: 299e7d8758e02815422a6fd13d6880a37d1f0ae6 |
Deploy preview created for package Built with commit: 5275b71534400d9f7867c9fe4682278c7dadf334 |
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.
Passes all the tests, and I don't see any stragglers still importing from internal/vendor.
Even after yarn clean
yarn install
and yarn build
, I am still seeing built files importing from those paths, but maybe that can be a followup?
I see one place where those are coming from. Fix incoming. |
Fixed in 86ef4f5. There was a build script for icons that was automating creation of modules that used an |
Looks good - should we remove the documentation around the vendor dirs? |
Addressed in e47e8c1 |
@kennylam There was a big merge conflict here after the CWC removal PR landed. It was caused by a lot of overlap in adjusting import statements. I've addressed the conflicts and this should now be ready to go. Note that we have a few more v2 PRs coming in the next week, after which we'll probably look for a new v2 release in the first or second week of Oct. |
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.
LGTM!
cf0b033
into
carbon-design-system:main
…tore (#12079) ### Related Ticket(s) Closes #12078 ### Description Turns out we were overzealous in #12010 by removing internal references to `@carbon/ibmdotcom-services-store`. This package has to work this way b/c its not published to npm. ### Testing instructions The best, most representative way I've found to test this is to build the project, then use `npm pack` to get a tarball, then install the tarball in a target application that bundles one or more modules in the `web-components/es/` folder, which itself uses modules from `@carbon/ibmdotcom-services-store`. 1. Checkout this branch locally, and run `yarn build` 2. Run `npm pack` ``` $ cd packages/web-components $ npm pack --pack-destination=/Users/m4olivei/Desktop ``` 3. Clone this test appilcation, or any other app that uses `@carbon/ibmdotcom-web-components` v2.14.0-rc.0. For example https://github.com/m4olivei/carbon-webpack-play/tree/services-store 4. Run the following in the application ``` $ cd carbon-webpack-play $ npm install ~/Desktop/carbon-ibmdotcom-web-components-2.14.0-rc.0.tgz $ npm run build ``` 5. Should build clean ### Changelog **New** - Re-introduces copying compiled modules from `@carbon/ibmdotcom-services-store` to `web-components/src/internal` directory. <!-- React and Web Component deploy previews are enabled by default. --> <!-- To enable additional available deploy previews, apply the following --> <!-- labels for the corresponding package: --> <!-- *** "test: e2e": Codesandbox examples and e2e integration tests --> <!-- *** "package: services": Services --> <!-- *** "package: utilities": Utilities --> <!-- *** "RTL": React / Web Components (RTL) --> <!-- *** "feature flag": React / Web Components (experimental) -->
Related Ticket(s)
Closes #12008
Description
Adjusts all
internal/vendor
imports to use the package imports instead.Changelog
Changed
[...]/internal/vendor/@carbon/[package][rest]
changed to@carbon/[package]/es[rest]
Removed