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

Upgrading to 3.7.0 #113

Closed
unematiii opened this issue Dec 16, 2020 · 16 comments
Closed

Upgrading to 3.7.0 #113

unematiii opened this issue Dec 16, 2020 · 16 comments

Comments

@unematiii
Copy link
Contributor

unematiii commented Dec 16, 2020

I'm having a bit of trouble upgrading to latest (after esm/cjs changes), some of my tests (ts-jest) now fail with:

 /Users/mati/xxx/node_modules/fastify-decorators/testing/index.js:8
    export { configureControllerTest, } from './configure-controller-test.js';
    ^^^^^^

    SyntaxError: Unexpected token 'export'

    > 1 | import { configureServiceTest } from 'fastify-decorators/testing';

Linting tests also fails:

/Users/mati/xxx/tests/unit/yyyy/services/zzz.service.spec.ts
  1:1   error    Resolve error: Cannot find module '/Users/mati/xxx/node_modules/fastify-decorators/testing/index.cjs'
    at createEsmNotFoundErr (internal/modules/cjs/loader.js:907:15)
    at finalizeEsmResolution (internal/modules/cjs/loader.js:900:15)
    at resolveExports (internal/modules/cjs/loader.js:432:14)
    at Function.Module._findPath (internal/modules/cjs/loader.js:472:31)
    at findModulePath (/Users/mati/xxx/node_modules/eslint-import-resolver-alias/index.js:99:27)
    at Object.exports.resolve (/Users/mati/xxx/node_modules/eslint-import-resolver-alias/index.js:75:10)
    at v2 (/Users/mati/xxx/node_modules/eslint-module-utils/resolve.js:117:23)
    at withResolver (/Users/mati/xxx/node_modules/eslint-module-utils/resolve.js:122:16)
    at fullResolve (/Users/mati/xxx/node_modules/eslint-module-utils/resolve.js:139:22)
    at relative (/Users/mati/xxx/node_modules/eslint-module-utils/resolve.js:84:10)  import/namespace

I'm not entirely sure what needs to be changed here... Any help will be appreciated.


Update:

Might not be related, but in package.json I can see

    "./testing": {
      "import": "./testing/index.js",
      "require": "./testing/index.cjs"
    }

however, there is no index.cjs present...

L2jLiga added a commit that referenced this issue Dec 16, 2020
 ### Fixed

 - Missed CJS files for `fastify-decorators/testing` (#113)
@L2jLiga
Copy link
Owner

L2jLiga commented Dec 16, 2020

My bad, forgot to add fastify-decorators/testing to rollup config, published v3.7.1

@unematiii
Copy link
Contributor Author

Thanks! That removed the linting errors, but my tests still fail.

@unematiii
Copy link
Contributor Author

Okay managed to get rid of SyntaxError: Unexpected token 'export' error with custom ts-config ("allowJs": true, "module": "ESNext") and using transform and transformIgnorePatterns in jest config. Hovever, now all async service tests fail with:

TypeError: Cannot read property 'then' of undefined

      44 |   describe('initializer', () => {
      45 |     it('initializes db client and attaches logger functions for knex `query-response` and `query-error` events', async () => {
    > 46 |       const service = await configureServiceTest({
         |                             ^
      47 |         service: DatabaseService,
      48 |       });

@unematiii
Copy link
Contributor Author

Looks like in node_modules/fastify-decorators/testing/configure-service-test.js, at following lines

if (promise == null)
                    promise = hasAsyncInitializer(service) ? readyMap.get(service).then(() => target) : Promise.resolve(target);

hasAsyncInitializer returns true but next call to readyMap.get(service) evaluates to undefined which results in error...

@L2jLiga
Copy link
Owner

L2jLiga commented Dec 16, 2020

Are you building ESM?

@L2jLiga
Copy link
Owner

L2jLiga commented Dec 16, 2020

I weren't able to make work ESM+TS+Jest so in one of my projects I switched to tap and mocha in ESM example

@L2jLiga
Copy link
Owner

L2jLiga commented Dec 16, 2020

Hope in Jest 27 it will work

@unematiii
Copy link
Contributor Author

unematiii commented Dec 16, 2020

I am building for commonjs and everything works, except for tests. So I created a separate tsconfig for tests:

"compilerOptions": {
    "lib": [
        "es2019",
        "es2020.promise",
        "es2020.bigint",
        "es2020.string"
    ],
    "module": "esnext",
    "target": "es2019",
    "strict": true,
    "esModuleInterop": true,
    "skipLibCheck": true,
    "forceConsistentCasingInFileNames": true,
    "baseUrl": "./",
    "paths": {
        "@/*": [
            "src/*"
        ]
    },
    "experimentalDecorators": true,
    "emitDecoratorMetadata": true,
    "noImplicitAny": true,
    "noUnusedLocals": true,
    "strictNullChecks": true,
    "allowJs": true,
    "moduleResolution": "node"
},

Now I just realized, that while configureServiceTest is loaded from ESM module ('.js'), service INITIALIZER and CREATOR are assigned in decorators/service.cjs and decorators/initializer.cjs respectively, hence, different instance of readyMap is used - I have no idea why....

Following apporach doesn't seem to work either:

jest.config.ts:

preset: 'ts-jest/presets/js-with-ts',
transform: {
  '^.+\\.(j|t)s?$': 'ts-jest',
},
transformIgnorePatterns: ['<rootDir>/node_modules/(?!fastify-decorators)'],

tsconfig.spec.json:

{
  "compilerOptions": {
        "lib": [
            "es2019",
            "es2020.promise",
            "es2020.bigint",
            "es2020.string"
        ],
        "module": "commonjs",
        "target": "es2019",
        "strict": true,
        "esModuleInterop": true,
        "skipLibCheck": true,
        "forceConsistentCasingInFileNames": true,
        "baseUrl": "./",
        "paths": {
            "@/*": [
                "src/*"
            ]
        },
        "experimentalDecorators": true,
        "emitDecoratorMetadata": true,
        "noImplicitAny": true,
        "noUnusedLocals": true,
        "strictNullChecks": true,
        "allowJs": true
    }
}

@unematiii
Copy link
Contributor Author

I weren't able to make work ESM+TS+Jest so in one of my projects I switched to tap and mocha in ESM example

Can you explain further what are the obstacles here?

@L2jLiga
Copy link
Owner

L2jLiga commented Dec 16, 2020

I'm going to create and investigate sample repository tomorrow, for a moment you can try to build your application and tests and then run tests on transpiled code or switch to another test framework (I think you would like avoid it)

@unematiii
Copy link
Contributor Author

I'm going to create and investigate sample repository tomorrow, for a moment you can try to build your application and tests and then run tests on transpiled code or switch to another test framework (I think you would like avoid it)

Thanks! Actually building tests and testing against dist might make sence. I can't afford to switch to another testing FW atm...

@unematiii
Copy link
Contributor Author

unematiii commented Dec 16, 2020

Update: I ended up reverting all the changes and kept my original ts-config (module: 'commonjs') by simply hard-mapping js to cjs in jest.config.ts:

moduleNameMapper: {
  // ...
  'fastify-decorators/testing': 'fastify-decorators/testing/index.cjs',
  // ...
},

@L2jLiga
Copy link
Owner

L2jLiga commented Dec 16, 2020

Very nice! I think this should be noticed here

@L2jLiga
Copy link
Owner

L2jLiga commented Dec 17, 2020

Here's PR about adding support for TS + ESM: jestjs/jest#10823, it's already merged but I didn't managed how to make it work yet, for a while I added section to documentation about configuring Jest

@unematiii
Copy link
Contributor Author

Thanks, will check this soon.

@unematiii
Copy link
Contributor Author

I think this can be closed now

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

No branches or pull requests

2 participants