-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add file extension for import statement #30
Conversation
This is for ESM. So the first step is to explore pros and cons (esp. potential blockers), and determine whether we want to upgrade to ESM (like azure-sdk core libs). Then this PR can be part of the task. |
@Eskibear The question here is to decide between CJS and ESM for the library. Currently, in the tsconfig file, we configured module as The con of ESM is the compatibility issue that some libraries or tools might not fully support ESM, especially if they were designed with CJS in mind. That's the reason why we will build the project in both CJS and ESM styles. In the readme of this repo, we specified the prerequisites to be Node.js LTS version. ESM has been supported natively in Node.js starting from 14.x. And 14.x has reached the end of its life. https://github.com/nodejs/release#release-schedule ESM is the standard module system in JavaScript and is the direction in which the whole JS ecosystem is moving. I don't worry the compatibility issue so much. The JS feature management library will focus on the client side story in the future. ESM is natively supported in modern browsers, which simplifies the setup for front-end projects. So I think our JS libraries should embrace ESM and encourage our users to do this. |
FeatureManagement-JavaScript/tsconfig.json Lines 3 to 6 in 122bb9b
It means TS compiler will generate JS code as ESM following Indeed the choice has been made since day 1, as we embrace ESM as much as possible.
Just note that existing Nodejs tools might handle ESM poorly than they declare. Just to make sure the generated CJS .js code works, in order not to block who are still using CJS (a huge portion). And one thing you probably need to decide, whether to update file extension from .js to .mjs/.cjs in generated code, esp in dist-esm. As nodejs runtime by default regards .js files as CJS modules if not explicitly specified by parameters. Overall, ESM makes things better, and we should go that direction if possible. |
Another interesting thing I found is that when I tried to use |
@Eskibear |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to go as it won't block anything.
No idea if tsc has compiler options forcing to check .js extension for relative specifiers. Or if there's linting rules for that. worth a try.
Why this PR?