Dan6erbond / sk-auth

Authentication library for use with SvelteKit featuring built-in OAuth providers and zero restriction customization!
MIT License
577 stars 70 forks source link

esm test build #26

Closed x-ror closed 3 years ago

x-ror commented 3 years ago

@Dan6erbond i added a test ESM build and update deps in sample app and sk-auth. let me know if u have any problems with it.

Dan6erbond commented 3 years ago

Great work! I will give this a look and evaluate if either of them perform better or work more reliably across various platforms.

As you can see, I just merged #27 which adds the build-esm dependency among others and currently the setup seems to work again.

It might be worth considering an additional ESM build using the package.json module field, this will require updates in client/package.json and providers/package.json as well as an additional build configuration for Rollup.

x-ror commented 3 years ago

@Dan6erbond ok, but are u sure that we need build-esm? it does nothing.

Dan6erbond commented 3 years ago

It seemed to have fixed the previous issues, but if it isn't needed I'll remove it again since the behavior is overall quite odd. See this comment by @SGarno.

Dan6erbond commented 3 years ago

Okay, I've looked over the changes, and it doesn't seem to work for me. I get the following output:

C:\<anonymous>\SvelteKitAuth\app\node_modules\sk-auth\dist\index.js:1
export { Auth as SvelteKitAuth } from './auth.js';
^^^^^^

SyntaxError: Unexpected token 'export'
    at wrapSafe (internal/modules/cjs/loader.js:979:16)
    at Module._compile (internal/modules/cjs/loader.js:1027:27)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at nodeRequire (C:\<anonymous>\SvelteKitAuth\app\node_modules\vite\dist\node\chunks\dep-e9a16784.js:68211:17)
    at ssrImport (C:\<anonymous>\SvelteKitAuth\app\node_modules\vite\dist\node\chunks\dep-e9a16784.js:68164:20)
    at eval (/src/lib/appAuth.ts:3:674)
x-ror commented 3 years ago

@Dan6erbond looks strange. seems like the project works with CJS on ur side ... but on my side - ESM (

Dan6erbond commented 3 years ago

Did you try the latest state on main with build-esm and clear all your dependencies to ensure we're on the same versions?

Seems Vite still has a lot to figure out with the way it uses ESBuild since it technically should prefer ES modules but they don't seem to work properly.

Edit: There might be a way to support both but I'd have to figure out how the best way would be to go about it since we need the nested file structure to continue working.

Dan6erbond commented 3 years ago

After some investigation since it seems both CJS and ESM builds are required to make things run smoothly for everyone.

I've upgraded the build pipeline in #32 as you can see, which features an ESM and CJS build in the same output directory, by renaming the ESM output to .esm.js files.

If you don't mind running a local test over it and reporting back. I'll be able to merge this and hopefully resolve the issues we've been seeing with Vite & Rollups tolerance towards dynamic modules.