[Jest] setupTests.js should always import regenerator-runtime/runtime?

[LBrian]

Do you want to request a feature or report a bug? A potential improvement, I could be wrong but happy to create a PR for this if folks agree with this.

What is the current behaviour? If config jest.transformIgnorePatterns for some modules require transformation, user will need to add import 'regenerator-runtime/runtime' in setupTest.js manually which we could potentially import by default as part of preact-cli dependency

If the current behaviour is a bug, please provide the steps to reproduce.

node@15.4.0

jest@26.6.3
preact-cli@3.0.5

added transformIgnorePatterns in package.json

 "jest": {
    "preset": "jest-preset-preact",
    "modulePaths": [
      "<rootDir>/src/"
    ],
    "transformIgnorePatterns": [
      "node_modules/(?!(wired-toggle|lit-element|wired-lib|lit-html)/)"
    ],
    "setupFiles": [
      "<rootDir>/tests/__mocks__/browserMocks.js",
      "<rootDir>/tests/__mocks__/setupTests.js"
    ]
  }

npm run test throws ReferenceError: regeneratorRuntime is not defined

added this solve the problem

import { configure } from 'enzyme';
import 'regenerator-runtime/runtime'. // fixed ReferenceError: regeneratorRuntime is not defined
import Adapter from 'enzyme-adapter-preact-pure';

configure({
    adapter: new Adapter()
});

What is the expected behaviour? Users shouldn't have to manually import the required transform dependencies

If this is a feature request, what is motivation or use case for changing the behaviour?

Please mention other relevant information.

Please paste the results of preact info here.

[rschristian]

Personally, I don't think this makes much sense. As users add configuration they become in charge of managing that configuration. If you want to add transforms then you need to ensure you have the dependencies installed for your needs.

Adding this as a dependency to the CLI solves what problem exactly? You can just add regenerator-runtime to your own project's dependencies; I don't see why we'd add another dependency so users could skip that step. That's what you need to do with every other library you'd like to use.

On top of that, Preact-CLI really has no opinion about your tests and leaves it up to you. The templates pretty much all give the same Jest + Enzyme suite, but the CLI has no involvement. You can choose whatever kind of test system that you want. Supplying regenerator would be an odd choice that would contrast that a fair bit.

Besides, I know the whole JetBrains suite of IDEs will throw warnings at this. They expect all imports to match the project's package.json.

[LBrian]

Personally, I don't think this makes much sense. As users add configuration they become in charge of managing that configuration. If you want to add transforms then you need to ensure you have the dependencies installed for your needs.

Adding this as a dependency to the CLI solves what problem exactly? You can just add regenerator-runtime to your own project's dependencies; I don't see why we'd add another dependency so users could skip that step. That's what you need to do with every other library you'd like to use.

On top of that, Preact-CLI really has no opinion about your tests and leaves it up to you. The templates pretty much all give the same Jest + Enzyme suite, but the CLI has no involvement. You can choose whatever kind of test system that you want. Supplying regenerator would be an odd choice that would contrast that a fair bit.

Besides, I know the whole JetBrains suite of IDEs will throw warnings at this. They expect all imports to match the project's package.json.

Adding import regenerator-runtime doesn't solve any problems (of course not creating problems) at all but makes generated templates more friendly for ppl to start implementing preact without spending time trying to Google and figuring out what's the error, isn't that the goal of CLI to generate templates and layout application structure for developers by default so they can dedicate time on developing instead of debugging?

Of course, developers can choose whatever testing frameworks/tools they want with Preact but CLI default generates templates for Jest + Enzyme to encourage developers to start with, I don't see why not to add in setupTest.js for Jest by default? They still can opt out anyway, can't really understand what is the logical problem you are trying to voice out.

the whole JetBrains suite of IDEs will throw warnings at this not everyone is using JetBrains, I personally use VSCode, so this is not really convincing to use one IDE case to rule everything out.

[rschristian]

The goal of Preact-CLI, as I see it, is to provide a solid jumping off point for creating PWAs with Preact. We can't support everything every developer might want to add, but we can provide a starting point. Hence why there is a focus on the lighthouse scores, of which a new Preact project will score 100's across the board. Where you take the project from there is up to you.

The problem with what you're proposing is that it goes against the general best practices and adds weight to everyone's experience. Firstly, that's just not a safe way to use dependencies. The user loses all control. Secondly, there is no way to "opt out" of a dependency; all users will have to download this dependency when they want to install Preact-CLI and maintainers will need to ensure it stays up-to-date. This also adds greatly to our surface area of valid issues, as if a user is having some troubles with regenerator, as the suppliers of regenerator (in the scenario you propse), it's something we'd need to look into fixing/helping with.

While you're absolutely right, not everyone is using JetBrains' tools, we do need to give a best-effort attempt at working with the most popular tools. We can't tell a whole group of people that they'll just have to deal with the warnings that their IDE will throw right out-of-the-box. That's not very courteous to them.

[rschristian]

The dependencies of a package, are the libraries that are strictly needed to ensure it can run as intended. As you can run Preact-CLI without regenerator-runtime, because Preact-CLI does not use regenerator-runtime, that should not be a dependency.

Hopefully that makes sense.

[LBrian]

The goal of Preact-CLI, as I see it, is to provide a solid jumping off point for creating PWAs with Preact. We can't support everything every developer might want to add, but we can provide a starting point. Hence why there is a focus on the lighthouse scores, of which a new Preact project will score 100's across the board. Where you take the project from there is up to you.

The problem with what you're proposing is that it goes against the general best practices and adds weight to everyone's experience. Firstly, that's just not a safe way to use dependencies. The user loses all control. Secondly, there is no way to "opt out" of a dependency; all users will have to download this dependency when they want to install Preact-CLI and maintainers will need to ensure it stays up-to-date. This also adds greatly to our surface area of valid issues, as if a user is having some troubles with regenerator, as the suppliers of regenerator (in the scenario you propse), it's something we'd need to look into fixing/helping with.

While you're absolutely right, not everyone is using JetBrains' tools, we do need to give a best-effort attempt at working with the most popular tools. We can't tell a whole group of people that they'll just have to deal with the warnings that their IDE will throw right out-of-the-box. That's not very courteous to them.

I agreed with you more now from the explanation (appreciated :grin: ) based on decoupling and less dependency is better point of views. Heuristically adding dependency for convenience could lead to unexpected results under different environments which likely increases maintenance effort.

I am a big fan of creating (still learning) high readability and easy-to-use tools/components and always assumed my audiences/users have zero knowledge about it, so I can keep pushing myself to rephrase/refactor in order to make it no brainer. Do you think it will be useful we can at least add comments in setupTests.js to let developers know they will need to import for transform?

// install/unmark the below line if you need Jest to transform modules
// import 'regenerator-runtime/runtime'

import { configure } from 'enzyme';
import Adapter from 'enzyme-adapter-preact-pure';

configure({
    adapter: new Adapter()
});

[rschristian]

I think that's outside of the scope. I get that this seems like a tiny thing to add, for what may be a common-ish need, but if we add comments for every common use case then the templates would be hundreds of lines of docs.

The error that's returned tells you exactly what's going on and tossing it into a search engine gives you the ways to go about addressing it. I think that's good enough. Docs on how to use Jest belong in the Jest ecosystem.

[rschristian]

Closing as I think the discussion has been resolved.