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

jest-transform-svelte is a bit funky #33

Closed
pngwn opened this issue Jun 9, 2019 · 14 comments
Closed

jest-transform-svelte is a bit funky #33

pngwn opened this issue Jun 9, 2019 · 14 comments

Comments

@pngwn
Copy link

pngwn commented Jun 9, 2019

The recommended Jest transform jest-transform-svelte, has a few issues that make it hard to recommend to svelte community.

The first issue is that it uses deasync to bundle using rollup in the transform. This is necessary because rollup's bundling is asynchronous but Jest transforms only support synchronous code. The issue with deasync is that is uses node-gyp which often causes problems for users running the windows operating system. Debugging problems with deasync (due to node-gyp) can be difficult and time-consuming for users.

The second issue is that there is no easy way to hook into the svelte and node-resolve plugins that jest-transform-svelte uses to bundle the code, this makes passing custom compiler options and svelte preprocessors very difficult (if it is possible at all).

The solution to both of these issues is to just use the svelte.compile method directly , compiling to cjs, allowing node to do its own thing regarding module resolution. This doesn't solve the use of custom preprocessors (which are asynchronous) but I have written a Jest transform that supports svelte preprocessors and custom compiler options. The solution is also a touch hacky but it uses only node APIs and doesn't require the use of node-gyp so it should give more consistent behaviour in different environments. Many production applications use svelte preprocessors (for sass/ less -> CSS usually but increasingly TS -> JS) and the lack of support for these in any transform make it essentially unusable for those users.

I don't have any strong recommendations here, just letting you know that the current transform has a few issues and a few limitations that won't present themselves until you try to configure the compilation process or test in a variety of environments.

@benmonro
Copy link
Member

benmonro commented Jun 9, 2019

Got any alternatives? Not sure if there's a call to action here but happy to discuss.

@pngwn
Copy link
Author

pngwn commented Jun 10, 2019

There aren't many options sadly. The only updated ones all use deasync (some bundle with rollup, others just preprocess with deasync) and the 'original' jest transform for Svelte just does a straight compile with no support for anything other than basic components (it also hasn't been updated).

I came across this problem when I was trying to write some testing documentation so I wrote the transform found in svelte-test to address these issues. It is working and has a series of tests to confirm. It was actually when I was switching from my own render utilities to svelte-testing-library that I found the issue with the default property.

@benmonro
Copy link
Member

hmm, ok. well closing for now, if you think of any options, feel free to re-open.

@silvestrevivo
Copy link

There is an option for this. Instead use svelte-jest, babel-jest and @babel/preset-env and change the jest.config.js for something like this:

module.exports = {
    transform: {
      "^.+\\.js$": "babel-jest",
      "^.+\\.svelte$": "svelte-jest"
    },
    moduleFileExtensions: ["js", "json", "svelte"],
    coverageReporters: ['html'],
    setupFilesAfterEnv: [
        "@testing-library/jest-dom/extend-expect"
    ],
};

Then, include a .babelrc file with the next config:

{
  "presets": [
    [
      "@babel/preset-env",
      {
        "targets": {
          "node": "current"
        }
      }
    ]
  ]
}

npm run test ("test": "jest src", inside scripts) and has to work!!

@felipesere
Copy link

Shold the docs be updated with this suggestion? I was not able to get it to work with the documentation but this worked

@mihar-22
Copy link
Collaborator

mihar-22 commented Dec 8, 2019

Hey @felipesere I'm not seeing the difference? What was suggested is taken from the docs, except we suggest a transformer now that also supports preprocessors. If you're referring to what silvestrevivo suggested.

@mihar-22
Copy link
Collaborator

mihar-22 commented Dec 8, 2019

Oh you might be referring to the the babel-jest part. There is a section that was added recently at the very bottom of the setup for Babel and preprocessors.

@pngwn
Copy link
Author

pngwn commented Dec 8, 2019

We will probably release an official Svelte transform at some point and point people to that from the Svelte docs. This way we won't need to rely on third parties for maintenance or proper documentation. I've personally spent a great deal of time supporting users using transforms that aren't fit for purpose so this will probably happen when we sort out some better documentation around testing.

The transform mentioned (twice) in this issue worked from the beginning with no issues and continues to do so.

@mihar-22
Copy link
Collaborator

mihar-22 commented Dec 8, 2019

If you're referring to svelte-test then the reason I didn't link to that is a few issues:

  1. In my testing (Svelte 3.16 and Node 12+) it doesn't work with preprocessors, it passes undefined or null to the svelte.preprocess call.
  2. Small issue but the repo was a little all over the place and couldn't make sense of the tests.
  3. It seems strange to pass options to the transformer through the globals property.
  4. I felt like it should just read the preprocess option from svelte.config.js to reduce repetition.
  5. Not sure how big this is but it was missing licensing.

I also found jest-transform-svelte flunky, and svelte-jest wasn't doing anything but passing in the component directly to the svelte compiler with no options or preprocessor support. Those are the reasons why I made svelte-jester.

@pngwn
Copy link
Author

pngwn commented Dec 9, 2019

The reason I wrote the svelte-test transform was to specifically work with preprocessors (without relying on node bindings) without bundling, which it does (I've been using it like this for over 6 months now). I'd need a repro to know why that might not be the case.

Using the jest config file is the defacto way to pass options to jest transforms because you have access to the config in the preprocess method. Using a non-standard svelte.config.js file avoids repetition if you have one. Svelte does not support a config file and probably never will, an official transform certainly won't be recommending random config files.

@mihar-22
Copy link
Collaborator

mihar-22 commented Dec 9, 2019

I'd love an official one and happy to contribute :) I wasn't referring to passing options to the transform through the config, but that the options in svelte-test are via the global prop which is not what it's used for. The docs shows how to pass options into the transformer.

I would definitely not put svelte.config.js in the category of random config files. Here is an open issue discussing it. I'm not sure why you think that it 'probably never will' support it. It'll help simplify bundler integrations and a shared config is obviously preferable in a bunch of cases. It's required right now by svelte-vscode. It's not officially supported but some form of shared config will be agreed upon by either Svelte officially or the community.

I do believe though the transformer should be able to load from the shared config or be passed in as an option, which I'll add in.

@RobbieTheWagner
Copy link

RobbieTheWagner commented Jun 8, 2020

I've been trying svelte-jest and svelte-test and both seem to not really work. I get errors like this: TypeError: _shepherdModal2.default is not a constructor. We need some kind of official solution, since deasync does not work for node 12+.

@mihar-22
Copy link
Collaborator

mihar-22 commented Jun 9, 2020

svelte-jester gets 1400+ downloads/week and no reported issues. It's the best you're going to get right now until async transformations are supported by Jest. You error is not related to either library, it has something to do with Jest and potentially a mistake in your code. I can't tell exactly what's causing it without more information. Feel free to raise a new issue.

@mihar-22
Copy link
Collaborator

mihar-22 commented Jun 9, 2020

Locking this thread. For any new readers feel free to use svelte-jester for now and follow this PR over at Jest to see when async transformations are ready.

@testing-library testing-library locked as resolved and limited conversation to collaborators Jun 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants