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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Object-Import Errors #1823

Merged
merged 1 commit into from Jun 15, 2020
Merged

Fix Object-Import Errors #1823

merged 1 commit into from Jun 15, 2020

Conversation

manuth
Copy link
Contributor

@manuth manuth commented Jun 12, 2020

The changes I've made in PR #1785 caused object-imports to fail.
This happened because the order-rule is not able to handle null as an import-name and I totally forgot to add tests for object-imports.

This PR will fix the support of object-imports by introducing a new import-type in addition to require and import: The import:object-type.

In order to match the order-rule's behavior perfectly, the object-literal being imported acts as the name which means following:

import log = console.log; // name: "console.log"
import debug = console.debug; // name: "console.debug"

Additionally there is a new import-group you can set for the order-rule called "object", which includes all object-imports.

This new "object" group is in the pathGroupsExcludedImportTypes-array by default.

This PR will fix #1821 and will fix #1808
Sorry for causing trouble 馃槄

Just like ordinary `import x =` expressions, `export import x =` expressions can come with a number of different module-references.
Either a require-expression such as `export import fs = require("fs")`, a literal such as `export import Console = console;` or an object-path `export import log = console.log`.

This means, that the `isExport` property merely says whether the `TSImportEqualsDeclaration` has a leading `export`, but not what the `moduleReference` looks like.

----

This arguably is a semver-minor, but since it should have been included in #1785, I'm calling this a bugfix.

Fixes #1821. Fixes #1808.
@manuth manuth changed the title Fix Object-Import Support Fix Object-Import Errors Jun 12, 2020
@coveralls
Copy link

coveralls commented Jun 12, 2020

Coverage Status

Coverage increased (+0.04%) to 97.892% when pulling f5d95e8 on manuth:object-import into b22a183 on benmosher:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.08%) to 97.721% when pulling 017c18e on manuth:object-import into 54eb51b on benmosher:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.08%) to 97.721% when pulling 017c18e on manuth:object-import into 54eb51b on benmosher:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.08%) to 97.721% when pulling 017c18e on manuth:object-import into 54eb51b on benmosher:master.

docs/rules/order.md Outdated Show resolved Hide resolved
docs/rules/order.md Outdated Show resolved Hide resolved
src/rules/order.js Outdated Show resolved Hide resolved
src/rules/order.js Outdated Show resolved Hide resolved
tests/src/rules/order.js Outdated Show resolved Hide resolved
@manuth manuth requested a review from ljharb June 13, 2020 11:25
@ljharb ljharb merged commit f5d95e8 into import-js:master Jun 15, 2020
@manuth manuth deleted the object-import branch June 15, 2020 21:38
@manuth
Copy link
Contributor Author

manuth commented Jun 18, 2020

@ljharb do you still have a fair share of PRs to work on before the next minor or build version is released?
Having this change implemented would help me a lot working on my projects 馃槃

@ljharb
Copy link
Member

ljharb commented Jun 18, 2020

Not many; I鈥檒l look into a release soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants