Skip to content

Makes explicit imports that rely on index file inference #2368

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

Closed

Conversation

lukejacksonn
Copy link

@lukejacksonn lukejacksonn commented Jan 20, 2020

Partially resolves #2277 in combination with #2364

There are some import paths that rely on the bundler to resolve ./somefolder to ./somefolder/index.js at build time. This doesn't play well with the es module spec which demands import paths be complete.

Adding /index to these import statements (which only exist in the root index.js file) is minimally invasive and has no adverse effects on any of the existing builds but opens the door to a trivial transformation which adds .es.js extensions to import paths in order to generate a fully esm compatible build from source.

Since the project became dependency free the only manual edit to the source after this PR (in order to unlock browser esm compatibility) is to remove the unprotected use of process.env. I could pull in the changes from that commit to this PR or make a separate PR.

After that.. the babel transform (which exists here) would be all that is required.

@lukejacksonn lukejacksonn requested review from IvanGoncharov and leebyron and removed request for IvanGoncharov January 20, 2020 14:24
@IvanGoncharov
Copy link
Member

@lukejacksonn Thank 👍
The amount of work you put into #2277 motivated me to try to squeeze this feature into 15.0.0 release.
So I slowly going through changes you made in #2277 and trying to figure out how to merge them. For example, I don't want to merge /index without ESLint rule that verifies that we will not have regressions in the future.
Also, I want to keep *.d.ts files in sync so I opened #2377

@lukejacksonn
Copy link
Author

That is good to hear! So what you are saying is that this PR needs to be accompanied by one which adds an ES Lint rule which demands that developers always provide full paths (i.e. ./something/index rather than ./something)?

@IvanGoncharov
Copy link
Member

Fixed in #2377

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