-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
feat(jest-haste-map): support dependencyExtractor
written in ESM
#12008
Conversation
5c21bae
to
db74246
Compare
@@ -230,12 +231,16 @@ export default class HasteMap extends EventEmitter { | |||
return HasteMap; | |||
} | |||
|
|||
static create(options: Options): HasteMap { | |||
static async create(options: Options): Promise<HasteMap> { |
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.
code in general looks good, but this makes it a breaking change and must wait for Jest 28 🙂 I'll add it to the milestone so we don't forget
8cce621
to
f59420e
Compare
'a.js': 'module.exports = {foo: 5};', | ||
'dependencyExtractor.mjs': ` | ||
const DYNAMIC_IMPORT_RE = /(?:^|[^.]\\s*)(\\bdynamicImport\\s*?\\(\\s*?)([\`'"])([^\`'"]+)(\\2\\s*?\\))/g; | ||
export function extract(code, filePath) { |
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 not quite sure we should accept extract
named export or a default export here.
named export:
export function extract(code, filePath) {
const dependencies = new Set();
if (filePath.includes('skip-deps')) {
return dependencies;
}
const addDependency = (match, pre, quot, dep, post) => {
dependencies.add(dep);
return match;
};
code.replace(DYNAMIC_IMPORT_RE, addDependency);
return dependencies;
};
default export:
export default {
extract(code, filePath) {
const dependencies = new Set();
if (filePath.includes('skip-deps')) {
return dependencies;
}
const addDependency = (match, pre, quot, dep, post) => {
dependencies.add(dep);
return match;
};
code.replace(DYNAMIC_IMPORT_RE, addDependency);
return dependencies;
}
}
2030f21
to
7c30268
Compare
Hey! I've started to land breaking changes, so if you rebase this we can see where we're at? 🙂 |
7c30268
to
08e4923
Compare
Ok. I've done the rebase. |
Awesome, thanks! I think I've commented on the two open questions? |
e9abd9f
to
e2325bc
Compare
e2325bc
to
6a57b81
Compare
Co-authored-by: Simen Bekkhus <sbekkhus91@gmail.com>
e4de85a
to
55cd8e1
Compare
@@ -116,6 +116,56 @@ describe('--findRelatedTests flag', () => { | |||
expect(stderr).toMatch(summaryMsg); | |||
}); | |||
|
|||
test('runs tests related to filename with a custom dependency extractor written in ESM', () => { |
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.
@SimenB I am not sure if this is a valid e2e test of ESM dependency extractor. I just found a commonjs version above and did the same against ESM.
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.
that's fine 👍
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.
thanks!
dependencyExtractor
written in ESM
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
Part of #11167.
See the discussion on the
dependencyExtractor
config: #11167 (comment)Test plan
new HasteMap(...)
calls toawait HasteMap.create(...)
Runtime.createHasteMap(...)
calls toawait Runtime.createHasteMap(...)