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

[bug: #19, #22] Fix examples and v27 Jest support / ESM #23

Merged
merged 1 commit into from
Sep 9, 2021
Merged

Conversation

rossyman
Copy link
Owner

@rossyman rossyman commented Sep 9, 2021

Fix examples and v27 Jest support / ESM.

Changelog

  • Add new --jsdom flag / option
    • Enables the user to specify whether they want per-test environment config with @jest-environment data-block, or to default to jsdom within jest.config.json.
  • Ensure Jest DOM @types are only installed if both ts and jest-dom option are true
  • Add ESM specific configuration for Jest:
    • Use --experimental-vm-modules flag when running Jest
    • Enable useESM configuration property for ts-jest
    • Force .mjs transformer usage in jest.config.json for svelte-jester. See svelte-jester 2.x does not work #19 (comment)
    • Add .svelte extensions to Jest extensionsToTreatAsEsm; and .ts depending on the TS option
  • Improve example test files for a mixture of TS, JS and JS DOM configurations
  • Remove divergence of lib property within tsconfig.spec.json to extend from tsconfig.json

@rossyman rossyman added documentation Improvements or additions to documentation enhancement New feature or request labels Sep 9, 2021
@rossyman rossyman requested a review from braebo September 9, 2021 16:23
@rossyman rossyman self-assigned this Sep 9, 2021
@rossyman rossyman merged commit bd18366 into main Sep 9, 2021
@rossyman rossyman deleted the bug/gh-22 branch September 9, 2021 16:31
@@ -197,33 +200,50 @@ class SvelteJestAdder extends Adder {
Preset
.editJson('jest.config.json').merge({
transform: {
'^.+\\.svelte$': ['svelte-jester', {preprocess: true}],
'^.+\\.svelte$': ['./node_modules/svelte-jester/dist/transformer.mjs', {preprocess: true}],
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this not be just svelte-jester?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I just saw your explanation on #19 (comment)

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yeah it's a rather peculiar issue. I would've assumed that Jest would've somehow picked up the .mjs file automagically when the ESM flag is enabled, but it seems to be forcing the .cjs transformer usage.


test('shows proper heading when rendered', () => {
const { getByText } = render(index);
expect(getByText('Hello world!')).toBeInTheDocument();
Copy link
Contributor

Choose a reason for hiding this comment

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

I liked that this was testing the .svelte file before, but it was challenging that the content was not the same in the two SvelteKit templates. Perhaps we can update the SvelteKit templates so that there's some piece of shared content to solve that?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps we could just test for Sveltekit? That would pass both templates.

Copy link
Contributor

Choose a reason for hiding this comment

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

That sounds like a good solution to me!

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yeah so I can rework the tests slightly to just look for "Sveltekit".

For non Jest DOM users, we can just use the toBeDefined() matcher on the result of getByText(), and similarly continue to use toBeInTheDocument() for Jest DOM users -- does sound like a better solution?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, that sounds like a good idea

@benmccann
Copy link
Contributor

Just FYI, we're planning on bringing back CJS support to svelte-jester in svelteness/svelte-jester#71. I think that ESM is probably the better default for SvelteKit projects though, so this is still a good change. Possibly some people will want it as an option, but we can see how this goes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

generate test file option generates nothing svelte-jester 2.x does not work
3 participants