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 does not work with macros -- build babel config is not used for test #383

Open
arvigeus opened this issue Dec 19, 2019 · 14 comments
Open
Labels
help wanted Extra attention is needed kind: bug Something isn't working solution: workaround available There is a workaround available for this issue

Comments

@arvigeus
Copy link

Current Behavior

For example: Using styled-components/macros breaks the test. Using styled-components does not.

Suggested solution(s)

https://kulshekhar.github.io/ts-jest/user/config/babelConfig

Additional context

I can make a PR for this, but not sure how to tell it to find the babel config that tsdx uses

@ambroseus
Copy link
Contributor

ambroseus commented Dec 19, 2019

@arvigeus
Copy link
Author

@ambroseus I was thinking the same, but I will need to untangle the config from that function. Haven't done something like that before, should be fun

@ambroseus
Copy link
Contributor

@arvigeus need to implement tsdx eject (like in CRA) to eject all configs in separate folder.. joke

@arvigeus
Copy link
Author

My first (failed) attempt: https://github.com/arvigeus/tsdx/commit/83d26bf66ddd4b560734fbbbe91e31ba3a92f332

Problem 1: Does not actually work at all.
Problem 2: Does not read the babel config from root (not implemented)

@arvigeus
Copy link
Author

Another failed attempt here: https://github.com/arvigeus/tsdx/tree/bugfix-ts-jest-use-babel

It seems like the correct way, but I am missing something. Or I am entirely wrong again...

@ambroseus
Copy link
Contributor

I also think about how to make our dev life easy and avoid configs hell :)

@jaredpalmer jaredpalmer added the help wanted Extra attention is needed label Jan 4, 2020
@agilgur5
Copy link
Collaborator

agilgur5 commented Feb 2, 2020

@arvigeus have you tried just adding babelConfig: true to your jest.config.js? i.e. per the docs link:

// jest.config.js
module.exports = {
  // [...]
  globals: {
    'ts-jest': {
      babelConfig: true
    }
  }
};

If you did, did that still not work? So long as your babelrc has macros as a plugin, I think it should work, i.e.:

{
  "plugins": [
    "macros"
  ]
}

I'm not sure that re-using all of TSDX's internal babel config is necessary for resolving this. I'm also not sure that it's necessarily ideal to re-use that for tests, because the test environment is different.

But I do agree (as I've said in other issues) that it should be easier to re-use pieces of TSDX and a preset would be good to have (though TSDX changes it based on config options, so it's a bit complex).

And this use case probably makes sense to support internally as well with a PR, in my opinion. I'm not totally sure why your attempts don't work for the Jest config -- I do see that you set babelOptions.plugins to preset (should be babelOptions.presets)

@agilgur5
Copy link
Collaborator

agilgur5 commented Feb 3, 2020

Just experienced a similar problem in one of my packages, which works fine with tsdx build but then for tsdx test requires @babel/plugin-proposal-class-properties to be configured. tsdx test should definitely re-use at least some of the plugins like macros and class-properties and some others.

I had some issues with using ts-jest with babelConfig: true not working either, might be some bug there but didn't investigate too much. I'm currently migrating that library to TS so I was able to just use babel-jest for the JS files instead of try having ts-jest process them (though tsdx build processes the JS files fine).

@agilgur5 agilgur5 added scope: integration Related to an integration, not necessarily to core (but could influence core) kind: bug Something isn't working and removed scope: integration Related to an integration, not necessarily to core (but could influence core) labels Mar 9, 2020
@agilgur5 agilgur5 changed the title Jest does not work with macros Jest does not work with macros -- build babel config is not used for test Mar 16, 2020
@agilgur5 agilgur5 added this to the v0.14.0 milestone Apr 20, 2020
@zenflow
Copy link

zenflow commented Aug 22, 2020

So just to make it explicit for anyone looking for a workaround:
This approach here #383 (comment) basically works, just replace babelConfig: true with babelConfig: '.babelrc', since babelConfig: true seems to be broken.

@agilgur5 agilgur5 added the solution: workaround available There is a workaround available for this issue label Aug 31, 2020
@agilgur5
Copy link
Collaborator

agilgur5 commented Oct 9, 2020

Just jotting some implementation notes here:

Started work, found merging problems...

So I've started to work on creating a Babel preset similar to @arvigeus 's work (although looks like their fork/branch/commit is gone now, links are 404'ing), but the merging TSDX currently does in babelPluginTsdx is very different from how Babel itself does merging. Babel fully replaces any duplicate plugins/presets in overrides, which is what babel-merge does too.

Furthermore, if TSDX just requests users to use a TSDX preset (a la #634), then duplicate plugins or presets actually won't be merged at all, they'll be re-run a second time per babel/babel#8799 ... 😕
This is quite a conundrum as while the logic of a preset is more explicit and the merging is more consistent with how Babel and the ecosystem works, it breaks existing usage really heavily, where folks configure something like @babel/preset-env and it gets merged with TSDX's internal @babel/preset-env config 😕

i.e. If instead one had:

module.exports = {
  presets: [
    '@tsdx/babel-preset',
    ['@babel/preset-env', /* .... */ ],
  ],
}

There would actually be two instances of preset-env, the one inside @tsdx/babel-preset, and the one in the list. They won't merge.

The fix, I suppose, is to allow configuring all of the plugins or presets within @tsdx/babel-preset:

module.exports = {
  presets: [
    ['@tsdx/babel-preset', {
      '@babel/preset-env':  /* .... */,
    }],
  ],
}

but that's very breaking and users would have to know that that's possible; it's somewhat low-level. TSDX could warn on detection of a duplicate preset/plugin though 🤔 But it'd be difficult to tell if the duplicate were intentional or not (I assume it'd be unintentional in most cases, but not all). I'm not really sure what the optimal option is here... issues with all those options.

Partial Resolution likely possible

That being said, this doesn't need to be entirely solved to resolve this issue. Only a partial resolution is needed for this, which I think can be done. A full resolution is necessary for #634 though.

Misc Notes

Current detection/merge logic has some misses

Also of worth to note is that the current detection logic misses things like @babel/env and env, or, in general shortcut names, so TSDX isn't quite merging everything already, though it is for the most common and conventional option.

Most overrides were removed

Per #583 (comment) , I did remove most of the overrides TSDX did during merging in #795, but modules: false still remains.
We could try to detect this option and error out during build though. But not sure if ignoring may be preferable if someone is using it in a different context (e.g. test or something, idk). It's not the most important thing as that's really not an option one should use with TSDX or Rollup in general

Per-output options could be problematic

Per-output options, like the format conditional we use for lodash usage create a bit of a problem with a preset. Presets can be configured, but normally you only configure it once in babelrc. And babelrc's overrides only handles env, not arbitrary things like format.
We'd have to still leave this logic in, merging per format... but this creates an issue that there isn't consistency and that the preset itself isn't the full source of truth 😕

This is currently the only place it's used though, so perhaps it's avoidable -- have been looking into change this one option for #759 already

@dbrxnds
Copy link

dbrxnds commented Feb 4, 2021

Has anyone found a fix to this? I tried using #383 (comment) as a fix but this only gave me more errors. I am also using styled-components and attempting to get the macro working.

@zenflow
Copy link

zenflow commented Feb 4, 2021

@iDavidB I got it to work but it was quite a while ago.. I'm just looking at the setup in my project..
Maybe you need to add the @babel/preset-env to your .babelrc...
(Or since it was a while ago and I haven't updated packages, possibly something has changed and the fix doesn't work now for whatever reason.)

So try adding the @babel/preset-env to your .babelrc. I think without this, your babel config for testing doesn't use @babel/preset-env at all, and therefore might lack some necessary plugins (e.g. syntax plugins), which could be the source of your errors.

.babelrc

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

jest.config.js

module.exports = {
  globals: {
    'ts-jest': {
      babelConfig: '.babelrc',
    },
  },
}

Let me know if that does the trick!
Otherwise, let us know what particular errors you are getting.. It's pretty hard to guess your problem without knowing the errors..

@dbrxnds
Copy link

dbrxnds commented Feb 4, 2021

@zenflow Thank you very much! That did do the trick!

@tl-NASEEBULLAH-AHMADI

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed kind: bug Something isn't working solution: workaround available There is a workaround available for this issue
Projects
None yet
Development

No branches or pull requests

7 participants