Skip to content

Conversation

@mnaamani
Copy link
Member

Attempt to Fix #511

@mnaamani mnaamani mentioned this pull request May 29, 2020
@mnaamani
Copy link
Member Author

I'm convinced now there is something uniquely wrong with my local dev environment. Clearly I have some cached typescript definitions files interfering with the build

import { ApiProps } from "@polkadot/react-api/types";

import Transport from "../../transport";
import { Transport } from "../../transport";
Copy link
Member Author

Choose a reason for hiding this comment

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

I can clearly see by examining ../../transport/index.ts that there is a default export class statement which means this fix is incorrect, but I can't figure out how my IDE is not seeing the same thing :)

Screen Shot 2020-05-29 at 9 19 48 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I know what's the issue here. There is joy-utils/src/Transport (file) and joy-utils/src/transport (directory). Perhaps in some environments where paths are considered case insensitive import from file takes priority over the import from directory and hence the error.

@mnaamani
Copy link
Member Author

mnaamani commented May 30, 2020 via email

mnaamani added 4 commits June 1, 2020 11:29
On Mac is seems node import paths files take precedence over folder because of lack of
real case sensitivity in OSX file system. eg in this case: Transport.ts and ./transport (folder)

Added macos to github actions matrix to test and confirm
@mnaamani
Copy link
Member Author

mnaamani commented Jun 1, 2020

So the quick and dirty fix 2f2df78 confirms it is the issue you pointed out. I'm not sure there is an environment var to control the precedence of how node handles import paths when a file or folder match, but in this particular case its not really the issue, its actually the fact that it can't tell the different between transport.ts and Transport.ts and ./transport/ due to OSX filesystem case sensitivity.

I added a job to github workflows to build for OSX so we can spot such intricate "bugs" in future, but it would be better I think to follow a convention where we try to avoid having a file and folder with similar names that would cause this case to be problematic.

@mnaamani mnaamani marked this pull request as ready for review June 1, 2020 09:05
@mnaamani mnaamani requested a review from Lezek123 June 1, 2020 09:20
@mnaamani mnaamani mentioned this pull request Jun 1, 2020
Copy link
Contributor

@Lezek123 Lezek123 left a comment

Choose a reason for hiding this comment

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

The CI works well, I was able to reproduce the error in my fork.
This simple fix seems to be enough for now, I will rename the file or change the structure in this directory in some PR later (joy-utils needs some cleanup anyway, as it is currently partially the old joy-utils and patrially a work-in-progress joystream-js)

@mnaamani mnaamani merged commit 99c6e75 into Joystream:development Jun 2, 2020
@mnaamani mnaamani mentioned this pull request Jun 2, 2020
@mnaamani mnaamani deleted the pioneer/build-fixes branch June 10, 2020 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants