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.

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

Support loading ES Modules #304

Merged
merged 27 commits into from
Jun 4, 2023
Merged

Support loading ES Modules #304

merged 27 commits into from
Jun 4, 2023

Conversation

beerose
Copy link
Contributor

@beerose beerose commented Apr 28, 2023

Continuation of #283

src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
@tianyingchun
Copy link

it seems that this change aim to native esm with .mjs

but while our pckage.json declaretype:module with source file extention .ts,.js, it will as esm also.

@beerose beerose marked this pull request as ready for review May 30, 2023 11:23
@beerose beerose requested a review from d-fischer May 30, 2023 11:23
@beerose
Copy link
Contributor Author

beerose commented May 30, 2023

it seems that this change aim to native esm with .mjs

but while our pckage.json declaretype:module with source file extention .ts,.js, it will as esm also.

AFAIK that's something that typescript loader will take care of. @d-fischer can you confirm?

package.json Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
@beerose beerose requested a review from d-fischer May 31, 2023 14:55
@d-fischer d-fischer linked an issue Jun 4, 2023 that may be closed by this pull request
@d-fischer d-fischer merged commit 3e981dc into cosmiconfig:main Jun 4, 2023
9 checks passed
@beerose beerose deleted the local-esm branch June 4, 2023 19:07
@tianyingchun
Copy link

tianyingchun commented Jun 5, 2023

it seems has a bug here

const loadJs: LoaderAsync = async function loadJs(filepath) {
  try {
    const { href } = pathToFileURL(filepath);
    return (await import(href)).default;
  } catch (error) {
    return loadJsSync(filepath, null);
  }
};

if we have named export here it will always return undefined

maybe it should be changed to

const { href } = pathToFileURL(filepath);
const moduleData =  await import(href);
return moduleData.default.|| moduleData;

consider below case

consider below example
`valid.config.js`

```ts
import { defineConfig } from 'vite';

// it is `esm` module causeof type:`module`
export const test = defineConfig({
  cake: 'a lie',
});

it will aways return me undefined.

@beerose 

@d-fischer
Copy link
Member

d-fischer commented Jun 5, 2023

I don't see why it should load a non-default export from the file. Do you have any actual use case for this? I'm pretty sure that vite does not use cosmiconfig.

Please don't crosspost your issues. Continued in #308.

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.

support mjs extensions out of box
4 participants