-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
v2.23 breaking group spacing/ordering #2081
Comments
@lensbart so there's a few issues here. npm packages can, in fact, start with a capital letter - just not new ones. So, this is a really risky pattern to use for internal modules (see https://www.npmjs.com/package/JSONStream vs https://www.npmjs.com/package/jsonstream). Still, when i add this test case (but use |
When I modify the test case to use an actually invalid package name, everything passes. This suggests that this was in fact a bug fix, and the warning is correct. |
(Additionally, you probably want |
Thanks for this insight. In fact, the codebase doesn’t use a rule or pattern to disambiguate internal/external packages. It’s just a convention, all aliases are listed in |
@lensbart then you'd need the typescript resolver to kick those in; your config only has a babel and webpack one. |
Whoops, sorry again for the confusion that I’m causing on this Friday evening. In order to avoid repetition, I define my paths once in |
ahh ok, that does clear that up :-) are you saying that in v2.22, your tsconfig paths were recognized as "internal", but in v2.23, they're recognized as "external"? |
Yep! Not sure if I should be proud or embarrassed of this pattern, but my babel config looks as follows (abridged): // .babelrc.js
const { readFileSync } = require('fs')
const { resolve } = require('path')
const { parse } = require('json5')
const { filter, map } = require('ramda')
// Get module alias names from TypeScript configuration file
const alias = filter(
(value) => !value.endsWith('/*'),
map(
(value) => `./${value[0]}`,
parse(readFileSync(resolve('./tsconfig.json'))).compilerOptions.paths
)
)
module.exports = (api) => ({
plugins: [
['module-resolver', { alias, extensions: ['.ts', '.tsx'] }],
]
}) |
alrighty, while i'd still suggest another convention, this does seem to be an unintentional change. |
Seeing probably the same issue in the following form:
With the following piece of config: pathGroups: [
{
pattern: '@our-company/**',
group: 'external',
position: 'after',
},
...,
], |
We have similar problem: in version 2.23.4 (compared to previous version we used 2.22.1), imports that are TypeScript path aliases are sorted differently. Rule config:
Example file: import { IncomingHttpHeaders, IncomingMessage, ServerResponse } from 'http';
import { isLocalhost } from '@core/utils'; // in tsconfig, there is compilerOptions.paths configured Previously there were no warnings, now there are two:
|
Is this still an issue in v2.24.0? |
It seems to work for us 👍 🎉 |
@ljharb Sorry, it was a false joy. I forgot to remove the |
The following comes up as an error since upgrading to 2.23:
The codebase I’m working on uses the convention that aliases start with a capital letter (i.e. the second import statement belongs to the
internal
group). There should be a space between import groups. The error goes away when removing the empty line between both import statements.The text was updated successfully, but these errors were encountered: