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] Fixing typo in IDE section #1744

Merged
merged 1 commit into from Jun 1, 2020
Merged

Conversation

darkartur
Copy link
Contributor

@darkartur darkartur commented May 2, 2020

Fixes #1743

@coveralls
Copy link

coveralls commented May 2, 2020

Coverage Status

Coverage remained the same at 97.787% when pulling a1c969f on darkartur:master into 40ee069 on benmosher:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.739% when pulling 3e06a10 on darkartur:master into 92caa35 on benmosher:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.739% when pulling 3e06a10 on darkartur:master into 92caa35 on benmosher:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.739% when pulling 3e06a10 on darkartur:master into 92caa35 on benmosher:master.

@ljharb
Copy link
Member

ljharb commented May 2, 2020

Can you provide a test case, that would have prevented this from happening?

@darkartur
Copy link
Contributor Author

darkartur commented May 2, 2020

Can you provide a test case, that would have prevented this from happening?

I wanted, but I haven't found any similar cases in tests. It requires a big work to achieve this.

If I understand right test cases covers rules but don't covers IDE integrations.

I test this by running two files in row:

// Component/index.ts
export { default } from './Component.tsx';
// Component/Component.tsx
export default function Component() {
}

It fails when I run like this:

eslint ./Component/index.ts ./Component/Component.tsx 

But pass if run in opposite direction:

eslint ./Component/Component.tsx ./Component/index.ts

I don't sure that I can reproduce this behaviour in tests

@ljharb
Copy link
Member

ljharb commented May 2, 2020

Maybe I'm a bit confused; #1743 doesn't have much background. It shouldn't be possible for an IDE integration to do anything different than is done on the command line; if something works on the CLI but breaks in an IDE, the IDE itself is broken, full stop.

@darkartur
Copy link
Contributor Author

darkartur commented May 2, 2020

Maybe I'm a bit confused; #1743 doesn't have much background. It shouldn't be possible for an IDE integration to do anything different than is done on the command line; if something works on the CLI but breaks in an IDE, the IDE itself is broken, full stop.

Please take a look to update of my previous message

@ljharb
Copy link
Member

ljharb commented May 2, 2020

Interesting, thanks - I'd bet you could add that setup inside https://github.com/benmosher/eslint-plugin-import/tree/master/tests/files, and that might lead to reproducing it?

@darkartur
Copy link
Contributor Author

Interesting, thanks - I'd bet you could add that setup inside https://github.com/benmosher/eslint-plugin-import/tree/master/tests/files, and that might lead to reproducing it?

I can try, but how I can run two files in row in test?

@darkartur
Copy link
Contributor Author

Interesting, thanks - I'd bet you could add that setup inside https://github.com/benmosher/eslint-plugin-import/tree/master/tests/files, and that might lead to reproducing it?

It was easier then I thought. My example was wrong a bit. This is right example:

// Component/components.ts
export { default as Component } from './Component.tsx'; // this used somewhere else
// Component/Component.tsx
export default function Component() {
}
eslint ./Component/components.ts ./Component/Component.tsx // fails
eslint ./Component/Component.tsx ./Component/components.ts // pass

@darkartur
Copy link
Contributor Author

I have found another case and fixed it too but I think this function have even more issues.

@darkartur
Copy link
Contributor Author

darkartur commented May 12, 2020

@ljharb what else can I do to merge this?

@ljharb ljharb merged commit a1c969f into import-js:master Jun 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

[no-unused-modules] Typo in code
3 participants