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

fix(@stylexjs/babel-plugin): Support .js resolved file extension imports from .ts files #839

Merged

Conversation

beaumontjonathan
Copy link
Contributor

@beaumontjonathan beaumontjonathan commented Jan 5, 2025

What changed / motivation ?

In a TypeScript project using moduleResolution: 'node16' | 'nodenext', relative imports must include the file extension because of the Node.js ESM resolver requires file extensions.

Consider the following:

import * as stylex from '@stylexjs/stylex';
import { vars } from '../vars.stylex.js';
import { theme } from '../theme.stylex.js';

const styles = stylex.create({
  root: {
    backgroundColor: vars.primary,
  },
});

The babel plugin currently attempts to verify whether these relative import files actually exist. It does this by trying to resolve the import string relative to the source file. First by checking the import string itself, then checking each of the valid file extensions in turn (js,ts,tsx,jsx,mjs,cjs).

The case above is rejected because it will attempt to resolve e.g. ../vars.stylex.js, ../vars.stylex.js.ts but not ../vars.stylex.ts. This results in a Only static values are allowed inside of a stylex.create() call compile error.

This change first checks whether the file extension in the relative import string is one of the recognized extensions, and if so, will remove the extension before attempting to resolve the valid extensions.

Linked PR/Issues

Related to #769, which has a similar issue but with the ESLint plugin instead. I'm happy to take a look at the ESLint plugin if this change is accepted.

Additional Context

I've added a test for this, but not sure it matches the existing code style because it makes use of jest.mock(..) for @dual-bundle/import-meta-resolve. I don't love this, but I could not find any other test examples for moduleResolution: 'commonJS'.

Pre-flight checklist

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 5, 2025
@beaumontjonathan beaumontjonathan changed the title Babel plugin: Support .js resolved file extension imports from .ts files fix(@stylexjs/babel-plugin): Support .js resolved file extension imports from .ts files Jan 5, 2025
@nmn
Copy link
Contributor

nmn commented Jan 6, 2025

Thanks! Code changes look good. I'll do some final testing before merging.

@nmn nmn merged commit 06ffe7e into facebook:main Jan 11, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants