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

[Breaking change] babel-loader => ts-loader in web build builder 8.5.0 #1836

Closed
emilio-martinez opened this issue Sep 12, 2019 · 9 comments
Closed
Assignees
Labels

Comments

@emilio-martinez
Copy link
Contributor

emilio-martinez commented Sep 12, 2019

Please make sure you have read the submission guidelines before posting an issue

Expected Behavior

Being able to use all Typescript features.

Current Behavior

babel-loader doesn't support all Typescript features, e.g., const enum, like ts-loader does.

Failure Information (for bugs)

ERROR in /path/to/file.ts
Module build failed (from /path/to/repo/node_modules/babel-loader/lib/index.js):
SyntaxError: /path/to/file.ts: 'const' enums are not supported.

See also babel/babel#8741.

Steps to Reproduce

Just run a web project build that includes const enums.

Context

Please provide any relevant information about your setup:

Version of Nx used: 8.5.0

Relevant commit: 811c50b
Relevant PR: #1792

Other

The below are other potential breaking changes that are related only in that they were done in the same commit/PR. Happy to move these into separate issues if preferable. In no particular order:

@emilio-martinez emilio-martinez changed the title Use of babel-loader instead of ts-loader for web build builder is a 8.5.0 breaking change [Breaking change] babel-loader => ts-loader in web build builder 8.5.0 Sep 12, 2019
@emilio-martinez
Copy link
Contributor Author

Looked into the implications of this change more in detail.

Reading through the @babel/plugin-transform-typescript docs, especially the "Caveats" section, I'm surprised this change went in. The configuration between both setups is not even close—Babel seems to ignore several of the tsconfig.json options and requires to set Babel config equivalents instead. That's in addition to not supporting all the features, as mentioned in the description.

@emilio-martinez
Copy link
Contributor Author

cc @jaysoo @vsavkin

@jaysoo
Copy link
Member

jaysoo commented Sep 27, 2019

@emilio-martinez Hey, thanks taking the time to report this issue.

The main objective for moving to babel-loader is that babel is used in most non-Angular projects (React, Vue, etc.), so it makes sense to support the majority use case. This means we won't be moving back to ts-loader for the web builder.

As for differential loading and e2e tests, we've decided that it's in everyone's best interest to have differential loading on. It makes the experience much better for endusers on modern browsers, so there isn't a good reason to turn it off. The e2e tests were update reflect the changes for differential loading between previous ts-loader implementation vs the babel-loader implementation.


Since we won't be supporting ts-loader in web builder moving forward, I'm going to close this issue. If there is anything we can change to make the babel-loader experience better please let us know!

@jaysoo jaysoo closed this as completed Sep 27, 2019
@emilio-martinez
Copy link
Contributor Author

@jaysoo Thanks for the response. I'm not arguing for you to revert, and I appreciate you trying to justify your change, but what I'm simply calling out is that it's a breaking change.

In other words, you didn't follow semver, and now we're facing having to refactor internal and client applications that are breaking for what you published as a minor version bump of your lib.

Fortunately, the fix is simple—bump up a major version.

@emilio-martinez
Copy link
Contributor Author

By the way, our breaking projects are not Angular-based.

@emilio-martinez
Copy link
Contributor Author

emilio-martinez commented Sep 30, 2019

@jaysoo @vsavkin One more update, after refactoring to side-step the issues around declare class, I'm getting runtime errors in IE11 due to how babel transforms decorators. At this point, our codebases are blocked from upgrading.

@atifsyedali
Copy link

Our codebase broke wrt namespaces, const enums, decorators, and more.

@jaysoo Can't we have the Typescript compiler in transpileOnly mode pipe output to Babel, with the option of disabling Babel altogether? Then in parallel use the ForkTsChecker plugin.

This may seem like it may make the builds slower, but '@babel/preset-typescript' is already installed in babel, which you can now remove if you first pipe through tsc before babel.

@jaysoo
Copy link
Member

jaysoo commented Oct 17, 2019

@atifsyedali Hey, thanks for chiming in. I don't think it'll quite work with using transpileOnly since depending on what babel-plugins you use, it may not transpile using just tsc. I think we should be able to get support parity with prevous ts-loader setup.

@github-actions
Copy link

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants