chore: bump-schematics-deps#21009
Conversation
🚨 PR Title Validation Failed 🚨Your pull request title does not follow the required format. Please update it to match the expected pattern: Expected format: Allowed Types
Example of a valid PR title✅ ❌ Merge is blocked until the PR title is corrected. |
|
🚨 PeerDependencies Change Detected 🚨 Your pull request includes modifications to ❌ Invalid peerDependencies change in projects/schematics/package.json
- "parse5": "^7.2.1",
+ "parse5": "^8.0.0",Please note: Changes to peerDependencies are restricted and only permitted during framework update releases, as they may introduce breaking changes for customer's applications. |
|
☑ tests |
spartacus
|
||||||||||||||||||||||||||||
| Project |
spartacus
|
| Branch Review |
chore/CXSPA-11875
|
| Run status |
|
| Run duration | 03m 50s |
| Commit |
|
| Committer | Michał Gruca |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
4
|
|
|
0
|
|
|
0
|
|
|
101
|
| View all changes introduced in this branch ↗︎ | |
| }, | ||
| ], | ||
| }, | ||
| transformIgnorePatterns: ['node_modules/(?!parse5|@angular)'], |
There was a problem hiding this comment.
[QUESTION]
I can understand this line allows to transform just parse5 and @angular inside node_modules (rest of node_modules is not transformed).
Why do we need to add this config property at all?
- why do we need to transform parse5? I can see in major version 8.0 they changed to ESM-only feat: switch to ESM-only inikulin/parse5#1411 . But I'm still lacking understanding why would it (or something else?) imply that we have to transform it?
- Why did you add
@angularto the transform allow-list? To my limited understanding, before this PR,@angularwas ignored from transforming (as part of implicit default ignoring of allnode_modules.
There was a problem hiding this comment.
I've investigated answers to those questions and eventually I'd suggest the following code snippet:
| transformIgnorePatterns: ['node_modules/(?!parse5|@angular)'], | |
| transformIgnorePatterns: [ | |
| // Because ESM support is experimental in Jest, we don't enable it | |
| // So Jest has to transform ESM files inside node_modules. | |
| // For more see Jest docs: https://jestjs.io/docs/ecmascript-modules | |
| // | |
| // Due to a negative lookahead, we need to compose 2 patterns into one: | |
| // - Original pattern `node_modules/(?!(.*\\.mjs$|@angular/common/locales/.*\\.js$))` - copied from jest-preset-angular sources | |
| // see https://github.com/thymikee/jest-preset-angular/blob/8b7673ee739919433b0834fe4a3f55862801bd40/src/presets/create-cjs-preset.ts#L22 | |
| // - Custom pattern `node_modules/(?!parse5)` - because in parse5@18.0.0 became an ESM-only package | |
| `node_modules/(?!(.*\\.mjs$|@angular/common/locales/.*\\.js$)|parse5)`, | |
| ], |
My reasoning below:
I've tried removing this config on local in this PR branch and I can see the error:
Jest encountered an unexpected token
Jest failed to parse a file. This happens e.g. when your code or its dependencies use non-standard JavaScript syntax, or when Jest is not configured to support such syntax.
Out of the box Jest supports Babel, which will be used to transform your files into valid JS based on your Babel configuration.
By default "node_modules" folder is ignored by transformers.
Here's what you can do:
• If you are trying to use ECMAScript Modules, see https://jestjs.io/docs/ecmascript-modules for how to enable it.
• If you are trying to use TypeScript, see https://jestjs.io/docs/getting-started#using-typescript
• To have some of your "node_modules" files transformed, you can specify a custom "transformIgnorePatterns" in your config.
• If you need a custom transformation, specify a "transform" option in your config.
• If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option.
You'll find more details and examples of these config options in the docs:
https://jestjs.io/docs/configuration
For information about custom transformations, see:
https://jestjs.io/docs/code-transformation
Details:
/Users/krzysztof/code/classic-help-2/node_modules/parse5/dist/index.js:1
import { Parser } from './parser/index.js';
^^^^^^
SyntaxError: Cannot use import statement outside a module
1 | import { SchematicsException, Tree } from '@angular-devkit/schematics';
> 2 | import { DefaultTreeAdapterMap, parse as parseHtml } from 'parse5';
| ^
I'd go for enabling enable ESM support in Jest, instead of transforming it, but only then I learned in the Jest docs ESM support is experimental. So better not use it for testing. 👍
Regarding 1) I find it justified then to transform parse5
Regarding 2) I find it justified to ignore also @angular, but not all of it. Learned that by overriding the whole property transformIgnorePatterns, we've overriden the default of the jest-preset-angular which was defined as this. The original pattern is more sophisticated. I wondered maybe we could compose both patterns as an array, but due to negative lookahead with AND operator it didn't work as expected. I mean I tried:
const { createCjsPreset } = require('jest-preset-angular/presets');
const cjsAngularPreset = createCjsPreset();
/*...*/
transformIgnorePatterns: [
...cjsAngularPreset.transformIgnorePatterns,
'node_modules/(?!parse5)',
],
and then one of those folders was ignored (while it shouldn't). This gotcha is described also in Jest docs
So eventually we need to compose "manually" those 2 patterns (the original from jest-preset-angualr and our custom one for parse5).
And IMHO we should add a code comment with explanation, as it's not obvious for a future newcomer developer why such a pattern.
There was a problem hiding this comment.
Thanks @Platonn for the deeper analysis!
Indeed, the pattern transformIgnorePatterns: ['node_modules/(?!parse5|.*\\.mjs$|@angular/common/locales/.*\\.js$)'], also works
…ore/CXSPA-11875 # Conflicts: # feature-libs/asm/jest.schematics.config.js # feature-libs/cart/jest.schematics.config.js # feature-libs/checkout/jest.schematics.config.js # feature-libs/customer-ticketing/jest.schematics.config.js # feature-libs/estimated-delivery-date/jest.schematics.config.js # feature-libs/order/jest.schematics.config.js # feature-libs/organization/jest.schematics.config.js # feature-libs/pdf-invoices/jest.schematics.config.js # feature-libs/pickup-in-store/jest.schematics.config.js # feature-libs/product-configurator/jest.schematics.config.js # feature-libs/product-multi-dimensional/jest.schematics.config.js # feature-libs/product/jest.schematics.config.js # feature-libs/qualtrics/jest.schematics.config.js # feature-libs/quote/jest.schematics.config.js # feature-libs/requested-delivery-date/jest.schematics.config.js # feature-libs/smartedit/jest.schematics.config.js # feature-libs/storefinder/jest.schematics.config.js # feature-libs/subscription-billing/jest.schematics.config.js # feature-libs/tracking/jest.schematics.config.js # feature-libs/user/jest.schematics.config.js # integration-libs/cdc/jest.schematics.config.js # integration-libs/cdp/jest.schematics.config.js # integration-libs/cds/jest.schematics.config.js # integration-libs/cpq-quote/jest.schematics.config.js # integration-libs/digital-payments/jest.schematics.config.js # integration-libs/epd-visualization/jest.schematics.config.js # integration-libs/omf/jest.schematics.config.js # integration-libs/opf/jest.schematics.config.js # integration-libs/opps/jest.schematics.config.js # integration-libs/punchout/jest.schematics.config.js # integration-libs/s4-service/jest.schematics.config.js # integration-libs/s4om/jest.schematics.config.js # integration-libs/segment-refs/jest.schematics.config.js # package-lock.json # package.json # projects/schematics/jest.schematics.config.js # projects/schematics/package.json # projects/schematics/src/ng-add/__snapshots__/index_spec.ts.snap
|
🚨 PeerDependencies Change Detected 🚨 Your pull request includes modifications to ❌ Invalid peerDependencies change in projects/schematics/package.json
- "parse5": "^7.2.1",
+ "parse5": "^8.0.0",Please note: Changes to peerDependencies are restricted and only permitted during framework update releases, as they may introduce breaking changes for customer's applications. |
|
🚨 PeerDependencies Change Detected 🚨 Your pull request includes modifications to ❌ Invalid peerDependencies change in projects/schematics/package.json
- "parse5": "^7.2.1",
+ "parse5": "^8.0.0",Please note: Changes to peerDependencies are restricted and only permitted during framework update releases, as they may introduce breaking changes for customer's applications. |
|
🚨 PeerDependencies Change Detected 🚨 Your pull request includes modifications to ❌ Invalid peerDependencies change in projects/schematics/package.json
- "parse5": "^7.2.1",
+ "parse5": "^8.0.0",Please note: Changes to peerDependencies are restricted and only permitted during framework update releases, as they may introduce breaking changes for customer's applications. |
Merge Checks Failed |
|
🚨 PeerDependencies Change Detected 🚨 Your pull request includes modifications to ❌ Invalid peerDependencies change in projects/schematics/package.json
- "parse5": "^7.2.1",
+ "parse5": "^8.0.0",Please note: Changes to peerDependencies are restricted and only permitted during framework update releases, as they may introduce breaking changes for customer's applications. |
|
SAP employees are expected to use their SAP-email address for commits related to their work. Our compliance check has detected usage of an email other than a SAP one by a SAP employee. Please update your pull request accordingly. If you think this is wrong or need any assistance, please contact ospo@sap.com. |
|
🚨 PeerDependencies Change Detected 🚨 Your pull request includes modifications to ❌ Invalid peerDependencies change in projects/schematics/package.json
- "parse5": "^7.2.1",
+ "parse5": "^8.0.0",Please note: Changes to peerDependencies are restricted and only permitted during framework update releases, as they may introduce breaking changes for customer's applications. |
|
🚨 PeerDependencies Change Detected 🚨 Your pull request includes modifications to ❌ Invalid peerDependencies change in projects/schematics/package.json
- "parse5": "^7.2.1",
+ "parse5": "^8.0.0",Please note: Changes to peerDependencies are restricted and only permitted during framework update releases, as they may introduce breaking changes for customer's applications. |
Closes CXSPA-11874, CXSPA-11875, CXSPA-11876, CXSPA-11877
"jsonc-parser": "^3.2.0" -> "^3.3.1",
Changelog
"parse5": "^7.2.1" -> "^8.0.0",
Changelog
"semver": "^7.6.3" -> "^7.7.3",
Changelog
"ts-morph": "^26.0.6" -> "^27.0.2",
Changelog