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

Forward all parameters of transform to the upstream transfoemer #354

Merged
merged 1 commit into from
May 14, 2024

Conversation

kmagiera
Copy link
Contributor

@kmagiera kmagiera commented May 8, 2024

This PR fixes an issue where plugins parameter is not forwarded properly to the original transformer when svg-transformer is configured.

See https://github.com/facebook/react-native/blob/main/packages/react-native-babel-transformer/src/index.js#L184 for the full list of attributes of the object that upstream transformer takes. And specifically note that except from src, filename and options, also plugins is listed there.

Before this PR the list or attributes was limited to explicitely listed src, filename, and options, while plugins was missing. Since this transformer only uses src and filename directly, we replace the remaining part rest spread and forward it to the upstream transformer. This solution also makes this implementation more future proof in case new parameters are added.

kmagiera added a commit to software-mansion/radon-ide that referenced this pull request May 8, 2024
Fixes #198 

This is another approach at resolving the complexity of enabling
development JSX transforms.

As noted in #198 the old approach would sometimes produce an error that
the development transform throws when other instance of dev JSX
transform run, or when some of jsx-self or jsx-source transforms are run
on the same file along with the jsx dev version.

The problem in the repro provided in #198 was due to the use of quite
popular react-native-svg-transformer which doesn't forward `plugins`
attribute to the upstream transformer. I proposed a fix for this issue
in that repo
kristerkari/react-native-svg-transformer#354
however, it isn't practical to wait for a fix there and hence the PR
implements another workaround that doesn't depend on the `plugins`
attribute of the transformer.

The new approach relies on additional flag that is turned on when
non-dev version of the plugin is loaded. What we try to achieve is that
only jsx-dev transform is listed as plugin, and other varians (non-dev,
-self, and -source) versions are all turned off. Since we can't hijack
the plugins list, the only option is to use require overwrites.
Furthermore, we need to consider the case when someone has a proper
setup where only dev transform is listed and others are not. If we were
to only overwrite non-deb with dev version, that wouldn't suffice for
this particular case.

What we now do is:
1) disable -self and -source transforms entirely (read inline comment
for details on why we can do it)
2) replace non-dev JSX transform with dev version, and also use a flag
to mark when this replaced non-dev version is used
3) replace dev JSX transform with extra logic that checks for the flag
that marks whether non-dev version is used. If it is used, we return an
empty transform. Otherwise, we just return the dev transform as it means
that non-dev isn't registered.

As stated in the inline comment, this approach would break if someone
has dev transform registered before non-dev. Apparently, I don't see a
better way to move forward with this approach and I'd hope that the
current solution would cover the majority of setups.
@kristerkari
Copy link
Owner

Thanks!

@kristerkari kristerkari merged commit 01df699 into kristerkari:master May 14, 2024
@kmagiera kmagiera deleted the patch-1 branch June 11, 2024 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants