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

chore: remove .js in imports #1594

Closed
wants to merge 8 commits into from
Closed

chore: remove .js in imports #1594

wants to merge 8 commits into from

Conversation

weboko
Copy link
Collaborator

@weboko weboko commented Sep 21, 2023 •
edited
Loading

Problem

We have to manually mention .js in imports, when it's forgotten - the problem occurs.

Solution

Remove them as it is now allowed after the upgrade of TypeScript.

Notes

@weboko weboko requested a review from a team as a code owner September 21, 2023 09:47
@weboko weboko marked this pull request as draft September 21, 2023 09:48
@github-actions
Copy link

github-actions bot commented Sep 21, 2023 •
edited
Loading

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
Waku core 27.69 KB (0%) 554 ms (0%) 1.8 s (+46.49% 🔺) 2.4 s
Waku Simple Light Node 262.6 KB (0%) 5.3 s (0%) 4.1 s (-22.94% 🔽) 9.3 s
ECIES encryption 28.7 KB (0%) 574 ms (0%) 1.7 s (+0.03% 🔺) 2.3 s
Symmetric encryption 28.7 KB (0%) 575 ms (0%) 1.5 s (+6.25% 🔺) 2.1 s
DNS discovery 110.62 KB (0%) 2.3 s (0%) 3.2 s (+1.16% 🔺) 5.4 s
Privacy preserving protocols 117.38 KB (0%) 2.4 s (0%) 3.6 s (+35.03% 🔺) 5.9 s
Light protocols 25.83 KB (0%) 517 ms (0%) 1.4 s (+5.04% 🔺) 1.9 s
History retrieval protocols 24.9 KB (0%) 499 ms (0%) 1.4 s (+18.98% 🔺) 1.9 s
Deterministic Message Hashing 5.65 KB (0%) 113 ms (0%) 482 ms (+4.15% 🔺) 595 ms

@weboko weboko marked this pull request as ready for review September 21, 2023 22:14
weboko
weboko commented Sep 21, 2023
View reviewed changes
packages/core/karma.conf.cjs Outdated Show resolved Hide resolved
@fryorcraken
Copy link
Collaborator

I don't believe this is right. ESM states that imports must contains .js extension.
I don't think the bundle we output will be working.

I am guessing the issue is because moduleResolution in tsconfig was changed. Are we sure about this new Bundler setting?

@danisharora099
Copy link
Collaborator

I don't believe this is right. ESM states that imports must contains .js extension. I don't think the bundle we output will be working.

I am guessing the issue is because moduleResolution in tsconfig was changed. Are we sure about this new Bundler setting?

we should be okay in using Bundler as the moduleResolution as we use rollups for bundling;
with that resolution, we can safely discard .js suffixes: https://www.typescriptlang.org/tsconfig#moduleResolution

@weboko
Copy link
Collaborator Author

weboko commented Sep 25, 2023

Moving to draft for now as some this requires additional checking

@weboko weboko marked this pull request as draft September 25, 2023 23:50
@weboko weboko marked this pull request as draft September 25, 2023 23:50
@weboko weboko marked this pull request as draft September 25, 2023 23:50
@weboko
Copy link
Collaborator Author

weboko commented Feb 1, 2024

Closing for now

@weboko weboko closed this Feb 1, 2024
@weboko weboko deleted the weboko/up-imports branch February 1, 2024 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danisharora099 danisharora099 danisharora099 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

chore: get rid of .js in imports
3 participants
@weboko @fryorcraken @danisharora099