Skip to content
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

Fix for the jest.config.ts compiler to not interfere with tsconfig.json files #10675

Merged
merged 8 commits into from
Oct 22, 2020

Conversation

Gamote
Copy link
Contributor

@Gamote Gamote commented Oct 21, 2020

Summary

This PR resolves #10652.

The bug occurs when the consumer have a tsconfig.json file in the project defining the module configuration. In this case the internal compiler which takes care of processing the jest.config.ts automatically detect that. With this PR, I've added explicit definition on what module we want to use for output.

CHANGELOG.md file was updated to reflect these changes.

@Gamote Gamote changed the title Fix for the jest.config.ts compiler to not interfere with tsconfig.json file Fix for the jest.config.ts compiler to not interfere with tsconfig.json files Oct 21, 2020
Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a test as well? just adding a tsconfig file to the e2e test and seeing it still pass, perhaps?

CHANGELOG.md Outdated Show resolved Hide resolved
@Gamote Gamote requested a review from SimenB October 21, 2020 17:40
Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

I just noticed const DIR = path.resolve(__dirname, '../jest.config.ts'); - can you rename this to look like a directory instead of looking like a file? just

- const DIR = path.resolve(__dirname, '../jest.config.ts');
+ const DIR = path.resolve(__dirname, '../jest-config-ts');

or something.

Possibly stick it in tmp as well?

@@ -10,7 +10,7 @@ import {wrap} from 'jest-snapshot-serializer-raw';
import runJest from '../runJest';
import {cleanup, extractSummary, writeFiles} from '../Utils';

const DIR = path.resolve(__dirname, '../jest.config.js');
const DIR = path.resolve(__dirname, '../tmp/jest-config-js');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant

import {tmpdir} from 'os';`

😅

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SimenB If I move it to tmp the traverses directory tree up until it finds jest.config test will fail because it can't find the slash package.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should install slash in it, then. We have a runYarn helper already - I don't think it takes args, but that should be a simple refactor.

We can do it later, though 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah nice, runYarn is exactly what I was looking for

Copy link
Contributor Author

@Gamote Gamote Oct 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SimenB I've reverted the change with the tmp bacause I couldn't make runYarn to install the slash package. I even tried run('yarn install', DIR) with same module does not exist error.

As you said, we can do it later so we don't need to delay this fix.

Full example:

test('traverses directory tree up until it finds jest.config', () => {
  writeFiles(DIR, {
    '__tests__/a-giraffe.js': `
    const slash = require('slash');
    test('giraffe', () => expect(1).toBe(1));
    test('abc', () => console.log(slash(process.cwd())));
    `,
    'jest.config.ts': `export default {testEnvironment: 'jest-environment-node', testRegex: '.*-giraffe.js'};`,
    'package.json': '{ "devDependencies": { "slash": "^3.0.0" } }',
    'some/nested/directory/file.js': '// nothing special',
  });

  // 1st try with small local refactoring to allow args
  runYarn(DIR, undefined, ['install']);

  // 2nd try
  run('yarn install', DIR);

  const {stderr, exitCode, stdout} = runJest(
    path.join(DIR, 'some', 'nested', 'directory'),
    ['-w=1', '--ci=false'],
    {skipPkgJsonCheck: true},
  );

  // expects...
});

@SimenB SimenB merged commit 0eab327 into jestjs:master Oct 22, 2020
This was referenced Oct 26, 2020
This was referenced Mar 12, 2021
@github-actions
Copy link

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.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TS jest.config.ts error: SyntaxError: Unexpected token 'export'
3 participants