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

chore(lint): add no-const-enum rule #4677

Merged
merged 2 commits into from Jun 4, 2019
Merged

Conversation

cartant
Copy link
Collaborator

@cartant cartant commented Mar 30, 2019

Description:

This PR enables the no-const-enum rule that's in tslint-etc.

ATM, tslint-etc is used in the dtslint tests to catch malformed type expectations. This PR bumps the tslint-etc version and enables the above-mentioned rule in the project's tslint.json file.

The reason const enums cannot be used is outlined in #4556. Basically, using a const enum prohibits isolated module transpilation, as it's necessary to refer to another file to determine the literal that should replace a reference to a const enum member. That means that versions of RxJS that export const enums cannot be used with create-react-app-generated projects or any other projects that build using isolated modules.

As configured, the rule forbids all use of const enums. The rule does have an allowLocal option, but I don't see the use of local (i.e. within a single module) const enums to be a likely use case.

The main reason for adding the rule is to prevent the sort of breaking change that happened in 6.4.0 from happening again. In #4556, I mentioned that I tried to include a test that compiled the project using isolated modules, but was unable to do so (the reason is given in the linked PR).

The linting in this PR will fail until #4556 is merged.

Related issue (if exists): #4556

@coveralls
Copy link

Pull Request Test Coverage Report for Build 8298

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 10 unchanged lines in 5 files lost coverage.
  • Overall coverage decreased (-0.2%) to 96.677%

Files with Coverage Reduction New Missed Lines %
src/internal/operators/partition.ts 1 75.0%
src/internal/operators/repeatWhen.ts 1 95.71%
src/internal/OuterSubscriber.ts 1 50.0%
src/internal/observable/fromObservable.ts 3 0.0%
src/internal/operators/onErrorResumeNext.ts 4 81.82%
Totals Coverage Status
Change from base Build 8294: -0.2%
Covered Lines: 5208
Relevant Lines: 5387

💛 - Coveralls

@cartant
Copy link
Collaborator Author

cartant commented May 17, 2019

Aarrgghh! I rebased and force pushed the branch that I had on this notebook and managed to wipe my commits, as this notebook must have had a branch with no work in it. Or something. Will have to fix this later, when I get home. Hopefully, the work will be on my other notebook.

@benlesh benlesh merged commit 35c96cd into ReactiveX:master Jun 4, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jul 5, 2019
@cartant cartant deleted the no-const-enum branch September 24, 2020 07:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants