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

Defer <T>() => {} TSX error to Babel 8 #14367

Merged
merged 1 commit into from Mar 18, 2022

Conversation

nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Mar 17, 2022

Q                       A
Fixed Issues? Closes #14361
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

#14135 (which disallows <T>() => {} in TSX files, since it should be written as <T,>() => {}) caused problems for multiple users (at least 5 already reported it).

The reason is that most people enable the JSX and TS presets on all files, making also .ts be parsed as .tsx. This is wrong in theory, but until Babel 7.17.0 it always worked since TSX was just a superset of TS (except for the legacy <T> v type casts, but everyone uses v as T now). Even if #14135 is technically correct, it introduced a new error for everyone using single type arguments without a trailing comma and with that misconfiguration.

This error is correct so we should do it, but we could consider it as a breaking change and wait until Babel 8.

I'm marking this as i: discussion because while I think we should do it I'd accept a "we are doing the correct thing by raising the error, so we shouldn't revert it".

@nicolo-ribaudo nicolo-ribaudo added i: discussion PR: Revert ↩️ A type of pull request used for our changelog categories pkg: parser area: typescript labels Mar 17, 2022
@babel-bot
Copy link
Collaborator

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/51499/

@liby
Copy link

liby commented Mar 17, 2022

The reason is that most people enable the JSX and TS presets on all files, making also .ts be parsed as .tsx. This is wrong in theory, but until Babel 7.17.0 it always worked since TSX was just a superset of TS

So the correct configuration would be something like the following?

overrides: [
    {
        // Only forcibly enables jsx parsing for .tsx files
        test: /\.tsx$/,
        presets: [
            [
            "@babel/preset-typescript",
            {
                isTSX: true,
                allExtensions: true,
                allowDeclareFields: true,
            },
            ],
        ],
    },
    {
        test: /\.ts$/,
        presets: [
            [
            "@babel/preset-typescript",
            {
                allowDeclareFields: true,
            },
            ],
        ],
    },
],

But it always gives me a feeling of redundancy.

@nicolo-ribaudo
Copy link
Member Author

@liby Something like this is guaranteed to work:

module.exports = {
  overrides: [{
    test: /\.(tsx|ts)$/,
    presets: [["@babel/preset-typescript", { allowDeclareFields: true }]]
  }, {
    test: /\.(jsx|tsx)$/,
    presets: ["@babel/preset-react"]
  }]
};

@nicolo-ribaudo nicolo-ribaudo merged commit 4392e4f into babel:main Mar 18, 2022
@nicolo-ribaudo nicolo-ribaudo deleted the defer-tsx-generics branch March 18, 2022 18:07
CarsonF added a commit to SeedCompany/cord-field that referenced this pull request Mar 19, 2022
babel/babel#14367
I'd be fine with upgrading to accommodate this breaking change,
but Prettier can't figure out this out and tries to remove
the trailing commas I add.
We'll wait for the dust to settle.
@liby
Copy link

liby commented Mar 21, 2022

@nicolas-marien

Something like this is guaranteed to work

It doesn't work for me. The error message is:

ReferenceError: React is not defined

It works for me.

    overrides: [
      {
        // Only enable the React preset for .jsx and .tsx files
        test: /\.(jsx|tsx)$/,
        presets: [["@babel/preset-react", { runtime: "automatic" }]],
      },
      {
        // Only enable the TypeScript preset for .ts and .tsx files
        test: /\.(ts|tsx)$/,
        presets: [["@babel/preset-typescript", { allowDeclareFields: true }]],
      },
    ],

@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jun 20, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: typescript i: discussion outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser PR: Revert ↩️ A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug][Regression]: @babel/parse 7.17.7 - TS generics notation parsing fail when using react plugin
5 participants