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.

Sign up for GitHub

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

Jump to bottom

Handle unresolved JS import when type is exported with the same name #923

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Handle unresolved JS import when type is exported with the same name #923

wants to merge 1 commit into from

Conversation

Coobaha
Copy link

@Coobaha Coobaha commented Aug 31, 2017 •
edited
Loading

This was not giving any error

// fileA.js
export type MyType = {};

// fileB.js
import { MyType } from './fileA.js'

P.S. Some specs are failing in master?

1) ExportMap does not return a cached copy after modification:
2) exports-last valid
        const foo = 'bar'
        export default function foo () {
          const very = 'multiline'
        }
        export const bar = true
      :

ljharb
ljharb reviewed Sep 1, 2017
View reviewed changes
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this seems reasonable.

src/rules/named.js Outdated
const wrongType = meta && meta.exportKind !== undefined && meta.exportKind !== 'value'
if (wrongType) {
context.report(im[key],
im[key].name + ' not found in \'' + node.source.value + '\'')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably use a template literal

src/ExportMap.js Outdated
case 'InterfaceDeclaration':
m.namespace.set(n.declaration.id.name, captureDoc(docStyleParsers, n))
case 'InterfaceDeclaration': {
const meta = captureDoc(docStyleParsers, n)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe captureDoc should set the exportKind?

@benmosher
Copy link
Member

There have been enough Flow issues lately that I'm tempted to have ExportMap just ignore them completely, especially since Flow will complain if things are wrong... but if the merge conflicts resolve without test issues, I'll probably merge as-is for now. Thanks!

@ljharb ljharb added the flow label Apr 20, 2018
This was referenced Apr 12, 2019
Closed
Merged
@ljharb
Copy link
Member

ljharb commented Feb 1, 2021

@Coobaha i've rebased this, and around 10 tests are failing. When i comment out your implementation changes, only 2 tests are failing.

I know it's been awhile, but it'd be great to get this updated and landed :-) please mark it as ready for review once it's so.

@ljharb ljharb marked this pull request as draft February 1, 2021 17:26
@Coobaha Coobaha closed this Nov 9, 2023
@Coobaha Coobaha deleted the flowtype-wrong-import branch November 9, 2023 13:35
@ljharb
Copy link
Member

ljharb commented Nov 9, 2023

@Coobaha im not sure why you deleted the branch; it’d be great to get this in. Can you restore and reopen it?

@Coobaha Coobaha restored the flowtype-wrong-import branch November 9, 2023 17:03
@Coobaha Coobaha reopened this Nov 9, 2023
@Coobaha
Copy link
Author

Coobaha commented Nov 9, 2023

@ljharb Sorry was doing github cleanup and pruning stale branches. Branch is now restored

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ljharb ljharb ljharb left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
flow
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Coobaha @benmosher @ljharb