-
Notifications
You must be signed in to change notification settings - Fork 5
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
Conversation
@@ -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}], |
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.
Can this not be just svelte-jester
?
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.
Ah, I just saw your explanation on #19 (comment)
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.
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(); |
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 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?
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.
Perhaps we could just test for Sveltekit
? That would pass both templates.
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 sounds like a good solution to me!
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.
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?
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.
yeah, that sounds like a good idea
Just FYI, we're planning on bringing back CJS support to |
Fix examples and v27 Jest support / ESM.
Changelog
--jsdom
flag / option@jest-environment
data-block, or to default tojsdom
withinjest.config.json
.@types
are only installed if bothts
andjest-dom
option are true--experimental-vm-modules
flag when running JestuseESM
configuration property forts-jest
.mjs
transformer usage injest.config.json
forsvelte-jester
. See svelte-jester 2.x does not work #19 (comment).svelte
extensions to JestextensionsToTreatAsEsm
; and.ts
depending on the TS optionlib
property withintsconfig.spec.json
to extend fromtsconfig.json