Create empty mocks for jest #303
Conversation
… package.json file
| const path = require('path'); | ||
|
|
||
| const projectDirectory = path.resolve(`${__dirname}${path.sep}..${path.sep}..${path.sep}..`); | ||
| const testDirectory = path.join(projectDirectory, 'test'); |
There was a problem hiding this comment.
This is overall a really interesting solution, and it'd be wonderful if there was an established standard in providing mocks to dependents.
Anyway, this assume's the dependent's directory structure, and overall doesn't feel right to "inject" source files into a dependent, versus link to them
What if the mocks lived in a file here, and you just link to that in projectJson.jest.setupFiles?
Eg:
packageJson.jest.setupFiles.push('node_modules/appcenter-crashes/jest/mocks.js')| } | ||
|
|
||
| fs.writeFileSync(`${testDirectory}/${setupFileName}`, ` | ||
| jest.mock('NativeModules', () => ({ |
There was a problem hiding this comment.
How does jest.mock handle the scenario where the user has already defined a mock for NativeModules? Does this override/conflict with that? Or simply merge them?
| console.log('Could not read package.json file'); | ||
| return; | ||
| } | ||
| const projectJson = JSON.parse(packageJsonContent); |
There was a problem hiding this comment.
nit - is there a reason to name this variable "projectJson" not "packageJson"?
There was a problem hiding this comment.
Just a typo, thanks!
|
@ajwhite @dhei |
| const path = require('path'); | ||
|
|
||
| const projectDirectory = path.resolve(__dirname, '..', '..', '..'); | ||
| const mockFileContent = ` |
There was a problem hiding this comment.
nit - If you move the backtick(`) to the beginning of line 6, then the generated mocks/appcenter-crashes.js file won't have a empty new line at the beginning of the file.
| if (!fs.existsSync(mocksDirectory)) { | ||
| fs.mkdirSync(mocksDirectory); | ||
| } | ||
| fs.writeFileSync(`${mocksDirectory}/${mockFileName}`, mockFileContent); |
There was a problem hiding this comment.
It still feels wrong to inject source code into a dependent's project. Let's also recognize not every project will have the assumed file structure.
It would be more natural to define the mocks in the appcenter module (eg: appcenter-crashes/jest/mocks.js) and either:
- Automatically add the file path to
packageJson.jest.setupFilesduringpostinstall - Allow the user to manually add the file path to
packageJson.jest.setupFilesand document this process in the README
Example:
// package.json
{
...
"jest": {
"setupFiles": [
"__test__/mocks/*.js",
// simply points to the mocks in the appcenter module
"node_modules/appcenter-crashes/mocks.js"
]
}
}But AppCenter should avoid injecting mocks into the dependent project. Just have the dependent point to the files located in app center's module.
There was a problem hiding this comment.
@bmourat let's adopt the solution described here, I would vote for the first approach:
Automatically add the file path to packageJson.jest.setupFiles during postinstall.
| if (!fs.existsSync(mocksDirectory)) { | ||
| fs.mkdirSync(mocksDirectory); | ||
| } | ||
| fs.writeFileSync(`${mocksDirectory}/${mockFileName}`, mockFileContent); |
There was a problem hiding this comment.
@bmourat let's adopt the solution described here, I would vote for the first approach:
Automatically add the file path to packageJson.jest.setupFiles during postinstall.
|
|
||
| // Check if package.json has jest as dependency | ||
| let packageJson = ''; | ||
| const packageJsonFile = path.join(`${projectDirectory}`, 'package.json'); |
There was a problem hiding this comment.
Approach seems ok now based on latest discussion.
However this is duplicated code, we can put shared script code in the linker, I mean the core package and use it here.
|
@bmourat Works well for npm, however it failed when I tested with yarn. The cause is yarnpkg/yarn#853 so will need to document the manual steps for Yarn users... |
Fixes second reason for #298. First was fixed here #301.
We could add this logic to postlink script but I think it makes more sense doing it after installation since it is not part of the linking process.