hypothesis / frontend-shared

UI components and styles for Hypothesis front-end applications
https://patterns.hypothes.is
5 stars 2 forks source link

Fix `@babel/plugin-transform-react-jsx-source` warning in tests #810

Open lyzadanger opened 1 year ago

lyzadanger commented 1 year ago

Running tests (via make test, e.g.) outputs this warning to the console:

WARN: 'Add @babel/plugin-transform-react-jsx-source to get a more detailed component stack. Note that you should not add it to production builds of your App for bundle size reasons.'
Chrome Headless 109.0.5414.119 (Mac OS 10.15.7): Executed 346 of 458 SUCCESS (0 secs / 0.48 secs)
WARN: 'Add @babel/plugin-transform-react-jsx-source to get a more detailed component stack. Note that you should not add it to production builds of your App for bundle size reasons.'
WARN: 'Add @babel/plugin-transform-react-jsx-source to get a more detailed component stack. Note that you should not add it to producti
Chrome Headless 109.0.5414.119 (Mac OS 10.15.7): Executed 458 of 458 SUCCESS (0.814 secs / 0.586 secs)

A few notes:

acelaya commented 1 year ago

The referenced plugin is part of @babel/preset-react. We do not have the plugin as a direct dependency of any of our projects.

~We do 🤔. At least I can see it in client and frontend-shared.~

Never mind. I thought you were saying we didn't have a direct dependency into @babel/preset-react, but I guess you meant that we don't have a direct dependency on @babel/plugin-transform-react-jsx-source.

acelaya commented 1 year ago

So, I have confirmed the same code that prints this warning is getting "triggered" in other projects, like client. Yes, I have added console.logs to preact/debug minified file 😂

However, in client it does not print this warning. I'm trying to find out if we have some kind of configuration suppressing certain logs which we don't have in frontend-shared.

The weirdest thing is that it does print other console.warn calls that I add, just not this one.

acelaya commented 1 year ago

This might be a bug on preact https://github.com/preactjs/preact/issues/3818

acelaya commented 1 year ago

After some more testing, it looks like those warnings are triggered by the Icon and SvgIcon tests, when covering that they throw an exception if an unknown icon is provided.

Commenting-out those two tests removes the warnings.

acelaya commented 1 year ago

This gets even weirder 😅

It seems this is not something new, but a warning that has been happening in tests for some time but has been explicitly "muted" in individual tests by mocking console.warn via sinon. That's why it's not consistent between projects.

Some examples:

That said, adding the babel plugin should fix the warning, I suppose, as this plugin is the one responsible to set __source. I will dig a bit further on this direction.

acelaya commented 1 year ago

Regarding enabling this plugin, it depends on @babel/preset-react configuration, which for us is

{
  // Turn off the `development` setting in tests to prevent warnings
  // about `this`. See https://github.com/babel/babel/issues/9149.
  development: false,
  runtime: 'automatic',
  importSource: 'preact',
},

This plugin is only enabled if development is true, but as the comment states, doing so brings other problems.

Trying to enable the plugin manually by adding it to plugins: [...] while runtime: 'automatic' results in an error (which is the documented behavior).

We cannot change the runtime because otherwise we cannot set importSource: 'preact'.

acelaya commented 1 year ago

After some more debugging, these are my findings so far: