[WNMGDS-3028] Updated Angular Example #3358
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jack-ryan-nava-pbc
added
Impacts: Documentation
jack-ryan-nava-pbc
requested review from
pwolfert,
kim-cmsds and
tamara-corbalt
examples/angular/src/app/modalDialogWrapper/dialog.component.html
Outdated
Show resolved
Hide resolved
pwolfert
reviewed
Dec 19, 2024
|
Button text change is working via latest changes here: #3391 @kim-cmsds @tamara-corbalt @pwolfert I'd appreciate another review so we can merge this. We'll get plenty of feedback later from CCXP. |
|
Do you get angular-related lockfile changes when you install? |
|
@pwolfert let me push up that lockfile change |
examples/angular/src/app/modalDialogWrapper/dialog.component.ts
Outdated
Show resolved
Hide resolved
pwolfert
reviewed
Jan 27, 2025
* Auto-update package file commands to use `npm run`
Used the `yarn-to-npm` package for this
* Check in the first version of a package-lock generated by `synp`
using the experimental `--with-workspace` flag
* Fix out-of-sync react type dependency in one child design system
* Deleted and recreated the package-lock.json because of problems
I was not able to move foreward with the auto-migrated lockfile because of errors with `node-gyp` compiling things. I couldn't figure out any way to work around it except to make a fresh start and let the problematic locked dependencies go. Deleting it worked, but now that means we have a brand-new set of resolved dependencies, and that means we'll need to do a lot of thorough testing. I guess we'd need to do thorough testing anyway.
I'm not sure how, but when I run `npm install`, it also updates the `yarn.lock` file. That will at least give us some insights into what dependencies changed. We'll be able to directly compare the original locked dependencies and what we have now by inspecting this difference.
Also need to note here that I had to do the initial npm installation with the `--legacy-peer-deps` flag.
* Update all yarn commands and references to yarn
* Fix a ts-node / chalk error by downgrading `chalk`
All of the scripts that use `ts-node` would give the following error:
```
The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("chalk")' call instead.
To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/Users/patrickwolfert/Projects/design-system/package.json'.
1 import c from 'chalk';
~~~~~~~
at createTSError (/Users/patrickwolfert/Projects/design-system/node_modules/ts-node/src/index.ts:859:12)
at reportTSError (/Users/patrickwolfert/Projects/design-system/node_modules/ts-node/src/index.ts:863:19)
at getOutput (/Users/patrickwolfert/Projects/design-system/node_modules/ts-node/src/index.ts:1077:36)
at Object.compile (/Users/patrickwolfert/Projects/design-system/node_modules/ts-node/src/index.ts:1433:41)
at Module.m._compile (/Users/patrickwolfert/Projects/design-system/node_modules/ts-node/src/index.ts:1617:30)
at Module._extensions..js (node:internal/modules/cjs/loader:1422:10)
at Object.require.extensions.<computed> [as .ts] (/Users/patrickwolfert/Projects/design-system/node_modules/ts-node/src/index.ts:1621:12)
at Module.load (node:internal/modules/cjs/loader:1203:32)
at Function.Module._load (node:internal/modules/cjs/loader:1019:12)
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:128:12) {
diagnosticCodes: [ 1479 ]
}
```
I tried changing the tsconfig in several ways. Chalk is used by a mind-boggling number of our dependencies and indirect dependencies. `npm explain chalk` produced over 81,000 lines of output, and `chalk` can be found in our `package-lock.json` file 418 times. The latest version causes problems with ts-node because it only has an ESM version, and ts-node is trying to convert it into CJS for Node. My hypothesis now is that yarn was previously resolving it to the older version, and that's why it worked.
I also saw that we were using an older tsconfig node16 base even though we're on node18 now. I made that dependency explicit.
* Yarn lockfile updated again from chalk changes
I'm taking the opportunity to check this in and write a commit message to confirm that it was indeed resolving to `chalk` 4.1.2 prior to this migration. I checked out the `main` branch and reinstalled and checked the `yarn why chalk` output. The version we were using at the root level, which was getting consumed by `ts-node` and the scripts, was `4.1.2`. That confirms my hypothesis.
* Fix resolution override for react-docgen-typescript
* Get Gatsby to resolve the right version of the `@mdx-js/react`
My dev notes:
NPM is correctly resolving `@mdx-js/react` for the doc site to the version specified in the doc site package file (1.6.22), and it’s putting a `packages/docs/node_modules/@mdx-js/react` in the docs package; however, when I go to build it, it’s using 3.0.0, which is the version resolved in the root node_modules (because that’s what Storybook wants)
I believe that it’s TypeScript that is importing the wrong one, because TypeScript gives me an error about it. I’ve confirmed that it’s using the wrong one at build time because it’s trying to use a function that existed in 1.6.22 but was removed in 3.0 and throws an error
I learned about a `tsc --traceResolution` tool, and it basically told me that it tried 1.6.22, didn’t find any type definitions, and decided to use the project root version instead because it had definitions
I really don’t know why it was working before. It seems like TypeScript was able to import ES modules before this upgrade and now can’t. It’s not like the 1.6.22 type definitions existed in the yarn version. So why is TypeScript only now complaining?
I’ve made progress and also not. I’ve gotten TypeScript and the Gatsby compiler to correctly resolve and use the local 1.6.22 version of `@mdx-js/react` for our direct imports in our docs src, but when our `gatsby-plugin-mdx` (which the npm workspace hoists to the root) tries to import the same thing and use a 1.6.22 feature, it’s resolving to the root (hoisted) version!
Fixed it by telling webpack to always resolve that package to the local version.
I also needed to update the tsconfig so it would import ES modules and not just CJS
Unfortunately `npm run start` still doesn't work because there's something else that is throwing an error. We're a little closer though.
* Fix Gatsby's linter crashing the development server because of outdated plugins
I forced Gatsby to use a stripped-down version of our eslint config instead of its own built-in config, which is very outdated and uses an old version of eslint and an old FlowType plugin that doesn't work. These were some error messages we were getting:
```
ERROR #98123 WEBPACK
Generating development JavaScript bundle failed
Failed to load plugin 'flowtype' declared in 'BaseConfig » /Users/patrickwolfert/Projects/design-system/node_modules/eslint-config-react-app/index.js':
Package subpath './lib/rules/no-unused-expressions' is not defined by "exports" in
/Users/patrickwolfert/Projects/design-system/node_modules/eslint/package.json
failed Building development bundle - 15.275s
ERROR in Failed to load plugin 'flowtype' declared in 'BaseConfig »
/Users/patrickwolfert/Projects/design-system/node_modules/eslint-config-react-app/index.js': Package subpath './lib/rules/no-unused-expressions' is not
defined by "exports" in /Users/patrickwolfert/Projects/design-system/node_modules/eslint/package.json
develop compiled with 1 error
```
* Fix misordered command causing CI failure
* Ah, these commands were still incorrect
* Switch from `ts-node` to `tsx` after running into more ts-node issues with scripts
After fixing the docs issues, our scripts were broken again. TSX just works and doesn't complain about modules vs cjs and things like that.
* All `npm run` scripts need an additional `-- ` when passing args to the underlying command
* Missed the `--` in a couple places
* Missed some places where I needed `--` in the `build-examples.ts` script
* Get rid of the `yarn.lock` file
* Remove yarnrc that we don't need anymore
* Add an undo command per Jack's feedback
* Fix release-script problem that Kim found
* Remove extra yarn lockfiles that Jack found
|
@kim-cmsds @tamara-corbalt I've added this ticket to address a bug I found with child accordion elements. I'd like to get this initial work merged in soonish so this doesn't continue to drift and grow. I think we'll release this in a 12.1.beta release for the CCXP team to consume. |
tamara-corbalt
approved these changes
Feb 6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
A substantial number of changes have been made to enable our custom, preactement-based web component build process, to generate web components that can function when nested inside of other web components, or framework components like those used in Angular. This particular pull request is both proof that things are working (yay!) and a test to see that our web components could be used in a production-like manner.
The examples here are drawn from our existing Angular example but augmented to allow wrapping a la discussions we've had with developers on the CCXP team. The changes here are designed to address concerns by demonstrating example patterns. Specifically we address:
angular.jsonfileA thing I need to do yet is update the README.md with some guidance on usage patterns.
What feedback am I looking for here? (listed in descending order of importance)
How to test
/examples/angularyarn buildyarn startlocalhost:4200Checklist
[WNMGDS-####] Titleor [NO-TICKET] if this is unticketed work.Type(only one) label for this PR, if it is a breaking change, label should only beType: BreakingImpacts, multiple can be selected.