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

[Dove] Drop reliance on legacy runtime compatibility settings #2748

Closed
FossPrime opened this issue Sep 13, 2022 · 5 comments · May be fixed by #3375 or #2665
Closed

[Dove] Drop reliance on legacy runtime compatibility settings #2748

FossPrime opened this issue Sep 13, 2022 · 5 comments · May be fixed by #3375 or #2665

Comments

@FossPrime
Copy link
Member

FossPrime commented Sep 13, 2022

Running the Quick Start with typescript defaults returns the following error when executed with tsc app.ts unless a tsconfig file is made and a handful of legacy compatibility features are turned on.

SyntaxError: The requested module '@feathersjs/koa' does not provide an export named 'bodyParser'

Reproduction: https://stackblitz.com/edit/node-lzf6x8?file=package.json,app.ts

Upon some debugging, body parser is not exported at top level... but in a property called default:

❯ tsx
Welcome to tsx v3.9.0 (Node.js v16.14.2).
Type ".help" for more information.
> import('@feathersjs/koa').then(console.log)
Promise { <unknown> }
> [Module: null prototype] {
  default: {
    Koa: [Getter],
    bodyParser: [Getter],
    koa: [Function: koa],
    parseAuthentication: [Getter],
    authenticate: [Getter],
    errorHandler: [Getter],
    formatter: [Getter],
    rest: [Getter]
  },
  __esModule: true,
  koa: [Function: koa],
  authenticate: [Function: authenticate],
  parseAuthentication: [Function: parseAuthentication],
  errorHandler: [Function: errorHandler],
  rest: [Function: rest],
  formatter: [Function: formatter]
}

Proposed Solutions:

  1. Use tsup to automatically wrap the typescript modules for cjs and esm import.
  2. Drop support for CommonJs projects in new versions and only build for ES modules

Workarounds

  1. Using esno
  2. manually pick and hunt for dependencies within the CJS package

Additional Notes

@FossPrime FossPrime changed the title [Dove] Drop reliance on synthetic default imports with in ES modules [Dove] Drop reliance on legacy runtime compatibility settings Sep 13, 2022
@daffl
Copy link
Member

daffl commented Sep 14, 2022

This looks like providing an ESM build which we've been trying to do in #2665 can't seem to get it working properly in an at least somewhat backwards compatible way. I'd be happy if someone wants to take on digging more into this.

@FossPrime
Copy link
Member Author

FossPrime commented Sep 14, 2022

This looks like providing an ESM build which we've been trying to do in #2665 can't seem to get it working properly in an at least somewhat backwards compatible way. I'd be happy if someone wants to take on digging more into this.

It's not a small job to it while ensuring backward compatibility. Each module in the mono repo is different.
I suggest accepting PR's for supporting ESM builds, one or two modules at a time. The most correct way to it that I know is defining all the exports that are used. That breaks builds when people access random files within our packages

Though again, one thing that would be a HUGE help, would be to encourage using .js extensions in TypeScript now.

@FossPrime
Copy link
Member Author

FossPrime commented Oct 28, 2022

Building a TypeScript version of feathers-memory

DTS Build start
src/index.ts(1,20): error TS2792: Cannot find module '@feathersjs/errors'.
src/index.ts(2,19): error TS2792: Cannot find module '@feathersjs/commons'.
src/index.ts(3,48): error TS2792: Cannot find module '@feathersjs/adapter-commons'.
src/index.ts(4,18): error TS2792: Cannot find module 'sift'.
src/index.ts(6,37): error TS2792: Cannot find module '@feathersjs/adapter-commons'.

PNPM is a great way to make pull requests for a few of these... there's just too much to do all at once. Of course there are 20 lines of training wheels I could add to make it all work, but birds don't ride bikes...

@daffl
Copy link
Member

daffl commented Oct 29, 2022

Note that the current Feathers memory has been moved into core as @feathersjs/memory. I just didn't want to add deprecation notices yet since it's not "official".

I'm fairly sure it should build as expected.

@FossPrime
Copy link
Member Author

FossPrime commented Oct 29, 2022

Note that the current Feathers memory has been moved into core as @feathersjs/memory. I just didn't want to add deprecation notices yet since it's not "official".

I'm fairly sure it should build as expected.

A mail forwarding a dress would have saved a lot of time... Nothing hinted at work being done on somewhere else, there are dove branches there. A mail forwarding address would be nice ;), even months before you move.

Vuetify is full of heads up notices... even though there is no stable up to date version, 20 months after Vue 3 became stable.

image

The repo, issue tickets and PR system need not be altogether... Those can be moved to the monorepo before this one is ready...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants