Skip to content

build: switch to modules#1663

Merged
XhmikosR merged 2 commits intomainfrom
xmr/modules
Apr 26, 2023
Merged

build: switch to modules#1663
XhmikosR merged 2 commits intomainfrom
xmr/modules

Conversation

@XhmikosR
Copy link
Copy Markdown
Member

@XhmikosR XhmikosR commented Mar 28, 2023

@korki43 do you have any opinion? Modules are inherently faster :)

@XhmikosR XhmikosR added the build label Mar 28, 2023
@XhmikosR XhmikosR changed the title Switch to modules build: switch to modules Mar 28, 2023
@kiehnek
Copy link
Copy Markdown
Contributor

kiehnek commented Mar 28, 2023

I prefer modules, mostly because I work with Typescript a lot in which they're the default.

I've always found the .mjs extension a bit weird. It's essentially still the same JavaScript so I think the file should keep the .js extension. I'm not too sure if the mjs extension is even necessary anymore.

Edit: It seems like the mjs extension is still necessary if you don't specify type: "module" in your package.json.

https://nodejs.org/api/modules.html#the-mjs-extension

@XhmikosR
Copy link
Copy Markdown
Member Author

The reason I went with mjs is so that we don't need to specify "type": "module" in package.json. I don't like mjs either but I don't think there's another way to do it?

@kiehnek
Copy link
Copy Markdown
Contributor

kiehnek commented Mar 28, 2023

Yes, I agree. After all these are just a few build scripts so changing the whole projects module system wouldn't be a good idea.

@XhmikosR
Copy link
Copy Markdown
Member Author

Cool, I'll try to do the same changes in the bootstrap repo and then land this one :)

@XhmikosR
Copy link
Copy Markdown
Member Author

I'll revisit this when I'm done with the upstream changes so that we have the same point of reference.

@XhmikosR XhmikosR reopened this Apr 26, 2023
@XhmikosR XhmikosR marked this pull request as ready for review April 26, 2023 19:35
@XhmikosR XhmikosR merged commit 3d5e1e7 into main Apr 26, 2023
@XhmikosR XhmikosR deleted the xmr/modules branch April 26, 2023 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

No open projects
Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants