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

Can't recognize "const" and generic. #53

Closed
olegKusov opened this issue Oct 9, 2020 · 13 comments · Fixed by #68
Closed

Can't recognize "const" and generic. #53

olegKusov opened this issue Oct 9, 2020 · 13 comments · Fixed by #68

Comments

@olegKusov
Copy link

code: fn() => {const = ...}
error: Unexpected "const"

code: props: {[key in keyof T]: [Readonly<T[key]>, (v: T[key]) => void] }
error: Expected "]" but found "T"

@privatenumber
Copy link
Owner

privatenumber commented Oct 10, 2020

Thanks for this issue @olegKusov

Do you mind providing more details? For example, your Webpack config. A minimal reproduction, preferably.

Also, this isn't valid JS:

fn() => {const = ...}

Do you mean to do something like:

const fn = () => {
    const a = ...;
}

@gothammm
Copy link

gothammm commented Oct 13, 2020

I'm facing the same issue, for example I get the const error for the following code:

const usePrevious = <T>(value: T) => {
  const ref = useRef<T>();
  useEffect(() => {
    ref.current = value;
  });
  return ref.current;
};

export default usePrevious;

@privatenumber
Copy link
Owner

@peek4y

I added that exact code as a test-case to confirm that it works.

@privatenumber
Copy link
Owner

Closing this as it seems resolved/stale.

@olegKusov if your problem still persists, feel free to open a new ticket with more context.

@lexanth
Copy link

lexanth commented Nov 12, 2020

I think the issue here is if you set the loader to tsx, it treats all typescript as tsx, which has implications for generics syntax.

It would be nice if the loader could choose what to do based on file extension, but as a workaround, I set up separate rules for ts and tsx, with different loader configurations:

        {
          include: /\.m?[jt]sx$/,
          use: {
            loader: 'esbuild-loader',
            options: { target: 'es2015', loader: 'tsx' },
          },
        },
        {
          include: /\.m?[jt]s$/,
          use: {
            loader: 'esbuild-loader',
            options: { target: 'es2015', loader: 'ts' },
          },
        },

@privatenumber
Copy link
Owner

@lexanth

I think the issue here is if you set the loader to tsx, it treats all typescript as tsx, which has implications for generics syntax.

Ah true. I'm able to reproduce the bug with the code snippet in a tsx file. Seems this behavior is by TypeScripts spec and is known on esbuild's end (ref 1, ref 2).

It would be nice if the loader could choose what to do based on file extension, but as a workaround, I set up separate rules for ts and tsx, with different loader configurations:

It used to try to behave this way, but it's actually very difficult to determine the appropriate file type based on file name. For example, Vue SFC files have a .vue extension, but can be TypeScript, SCSS, Pug depending on the vue-loader query applied to it.

Simply passing in loader makes this behavior a lot more predictable.

I'll update the docs to recommend setting up two different rules for ts and tsx.

Thanks!

@privatenumber
Copy link
Owner

Update:

Instead of making users add a rule for both ts and tsx, I added fallback logic so that it tries ts if tsx fails with a particular error: a713461

That way, you can continue to use the /\.tsx?/ test like ts-loader.

@kirbysayshi
Copy link

@privatenumber I believe I'm running into this same issue with v2.4.0, but the error I'm receiving is slightly different:

Error: Transform failed with 1 error:
/path/to/file.ts:1:33: error: Expected "}" but found ":"

The Code in question is something like:

export const exhaustiveSwitch = <V>(
  l: Level,
  options: { [key in Level]: V },
): V => {
  if (!(l in options)) {
    throw new Error(`Oops`);
  }
  return options[l];
};

So it seems the error message may not be predictable!

@privatenumber
Copy link
Owner

@kirbysayshi

Thanks for reporting another repro.

The fallback logic was added in 2.5.0 which was released just the other day so try seeing if that fixes it.

If not, would you mind opening another issue for it?

@olegKusov
Copy link
Author

@privatenumber I believe I'm running into this same issue with v2.4.0, but the error I'm receiving is slightly different:

Error: Transform failed with 1 error:
/path/to/file.ts:1:33: error: Expected "}" but found ":"

The Code in question is something like:

export const exhaustiveSwitch = <V>(
  l: Level,
  options: { [key in Level]: V },
): V => {
  if (!(l in options)) {
    throw new Error(`Oops`);
  }
  return options[l];
};

So it seems the error message may not be predictable!

изображение
try to make separate loaders for ts and tsx. for me it worked.

@privatenumber
Copy link
Owner

With 2.5.0, you should be able to just use tsx and it will fallback to ts if it fails to compile.

@kirbysayshi
Copy link

@privatenumber I'll try to find some time to try to reproduce using 2.5.0. But I don't think I was clear with my initial report! You're correct that I was not using 2.5.0 when I reported the issue. But I reported it after looking at the code for the soon to be 2.5.0 (develop) branch. I saw that the fix there does not handle the case I found. Unexpected does not appear in the error message I received, even though the root cause was attempting to parse TS as TSX. So I wasn't trying to say that the case was unhandled in 2.4.0, but that even the fix in 2.5.0 wouldn't handle all cases of TS-parsed-as-TSX errors. I'm sorry that I wasn't more explicit.

@olegKusov yes, that is a workaround that I used as well! But since I noticed that the intention of the code is to gracefully fallback to TS mode in the event of a TSX failure, I figured reporting the exception I found was worthwhile. Thanks for the confirmation, regardless!

@privatenumber
Copy link
Owner

Thanks @kirbysayshi. Fixed in 2.6.1

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 a pull request may close this issue.

5 participants