-
Notifications
You must be signed in to change notification settings - Fork 123
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
Improve ESM support by using import with file extensions #302
base: master
Are you sure you want to change the base?
Conversation
That looks vey cool, thank you @JumpLink! If tests are green, I'm going to merge that.
Yes, good idea, let's do that. |
I guess we have to exclude files in |
@marcj Can you the workflow? |
@JumpLink what do you mean? |
@marcj "1 workflow awaiting approval " |
oh, I'm sorry🤣 approved |
Codecov Report
@@ Coverage Diff @@
## master #302 +/- ##
==========================================
- Coverage 78.31% 76.94% -1.37%
==========================================
Files 169 122 -47
Lines 17672 14716 -2956
Branches 4615 3973 -642
==========================================
- Hits 13839 11323 -2516
+ Misses 3833 3393 -440
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
@marcj I find it difficult to test my changes locally, so I am not sure if the gui packages are working. E.g. When I tried to install the package |
@marcj I will be unavailable for a week now, but I can finish the PR after that, maybe you can collect feedback for me until then. I will then also try again to be able to test things locally. |
Please see the notes in DEVELOPMENT.md in the root folder:
|
ok, cool! Let me know if something doesn't work. I'm available 24/7 in Discord. Much easier there to solve such issues 👍 |
Signed-off-by: Pascal Garber <[email protected]>
Signed-off-by: Pascal Garber <[email protected]>
@marcj Can you rerun the workflow and then merge this PR? |
Signed-off-by: Pascal Garber <[email protected]>
Signed-off-by: Pascal Garber <[email protected]>
Signed-off-by: Pascal Garber <[email protected]>
…ces to tsconfig files
Signed-off-by: Pascal Garber <[email protected]>
Do you have any plans to merge this PR? I want to add deepkit support to my library, but I faced some errors when creating tests because each dependency makes me add another dependency as peer dependency so I don't get And from what I read, this is the solution to my problem hahaha |
Signed-off-by: Pascal Garber <[email protected]>
Signed-off-by: Pascal Garber <[email protected]>
Hi, is there anyhing I can help to make this merged faster? This is blocking my project from converting to ESM. |
@NexZhu maybe you can Help me top fix some Tests in the Branche of this PR |
Sorry, being very busy lately, will help when I'm able to |
Summary of changes
Add
.js
file extension to all internal imports.This fixes #161 (see latest comments).
For example, the error message without this fix looks like this:
By the way
I used the regex search in VSCode to find all internal imports without a file extension with this search term:
This worked very well, we could also build a small lint script with this regular expression to check if there are imports without file extension.
Relinquishment of Rights
Please mark following checkbox to confirm that you relinquish all rights of your changes: