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

Test fails with ReferenceError: _{method}_decorators is not defined when using custom decorators #4631

Closed
3 tasks done
Aadhisivam opened this issue Jul 28, 2023 · 5 comments · Fixed by #4771
Closed
3 tasks done
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil

Comments

@Aadhisivam
Copy link

Prerequisites

Stencil Version

4.0.2

Current Behavior

*.spec.tsx test fails when using custom decorators in the component.

image

Expected Behavior

Expect test should not fail due to decorators.

System Info

System: node 18.16.1
    Platform: darwin (22.5.0)
   CPU Model: Apple M1 Pro (8 cpus)
    Compiler: /stencil-demo/stencil-demo/node_modules/@stencil/core/compiler/stencil.js
     Stencil: 4.0.2 😈
  TypeScript: 5.0.4
      Rollup: 2.42.3
      Parse5: 7.1.2
      Sizzle: 2.42.3
      Terser: 5.19.1

Steps to Reproduce

Add a custom decorator and use within component, test script will fail with reference error.

Code Reproduction URL

https://github.com/Aadhisivam/stencil-demo/

Additional Information

No response

@ionitron-bot ionitron-bot bot added the triage label Jul 28, 2023
@rwaskiewicz rwaskiewicz self-assigned this Jul 28, 2023
@rwaskiewicz rwaskiewicz added the Bug: Validated This PR or Issue is verified to be a bug within Stencil label Jul 28, 2023
@ionitron-bot ionitron-bot bot removed the triage label Jul 28, 2023
@rwaskiewicz
Copy link
Member

Thanks! I'm going to label this to get it ingested into our internal issue tracking system!

@rwaskiewicz rwaskiewicz removed their assignment Jul 28, 2023
@rwaskiewicz rwaskiewicz changed the title Test fails with ReferenceError: _{method}_decorators is not defined when using custom decorators in stencil v4 Test fails with ReferenceError: _{method}_decorators is not defined when using custom decorators Jul 28, 2023
@rwaskiewicz
Copy link
Member

It looks like this bug can be traced back to v2.19.0 - updated the title of the bug accordingly

@Aadhisivam
Copy link
Author

FYI, it worked previously in stencil v3.0.1 with jest and jest-cli v26.0.1.

Sharing the dependencies of working tests, for your reference

"dependencies": {
    "@stencil/core": "3.0.1"
  },
  "devDependencies": {
    "@types/jest": "^25.2.3",
    "@types/node": "^16.18.11",
    "jest": "26.0.1",
    "jest-cli": "26.0.1",
    "puppeteer": "^20.7.3"
  },

@rwaskiewicz

@rwaskiewicz
Copy link
Member

Thanks @Aadhisivam! Upon closer inspection, it looks like this was introduced in v3.2.0.

@louis-bompart
Copy link
Contributor

louis-bompart commented Sep 10, 2023

After some troubleshooting, I do think the issues might either be in TypeScript or how the class decorator is expunged.
I managed to reproduce an identical behavior without Stencil here:
https://github.com/louis-bompart/typescript-standalone-troubles


Edit:

Outdated investigation After checking the code source of the `ts.factory.updateClassDeclaration`, I noticed that using `ts.factory.createClassDeclaration` and returning its results do work, as long as the Class is not being used.

I noticed the NodeFactory had two updates "mode", updateWithoutOriginal and updateWithOriginal, the former being used by default.

I tried to mimick updateClassDeclaration with the updateWithOriginal, and it seems to work, in both Minimal Reproducible Example.
Here's the patch I used for Stencil (4.2.0)

diff --git a/node_modules/@stencil/core/compiler/stencil.js b/node_modules/@stencil/core/compiler/stencil.js
index d4d0311..0b611e6 100644
--- a/node_modules/@stencil/core/compiler/stencil.js
+++ b/node_modules/@stencil/core/compiler/stencil.js
@@ -229782,10 +229782,13 @@ const visitClassDeclaration = (config, diagnostics, typeChecker, program, classN
   const updatedClassFields = handleClassFields(classNode, filteredMethodsAndFields);
   validateMethods(diagnostics, classMembers);
   const currentDecorators = retrieveTsDecorators(classNode);
-  return ts.factory.updateClassDeclaration(classNode, [
+  const newNode = ts.factory.createClassDeclaration([
     ...((_b = filterDecorators(currentDecorators, CLASS_DECORATORS_TO_REMOVE)) !== null && _b !== void 0 ? _b : []),
     ...((_c = retrieveTsModifiers(classNode)) !== null && _c !== void 0 ? _c : []),
   ], classNode.name, classNode.typeParameters, classNode.heritageClauses, updatedClassFields);
+  ts.setOriginalNode(classNode, newNode);
+  ts.setTextRange(classNode, newNode);
+  return newNode;
 };
 /**
  * Take a list of `ClassElement` AST nodes and remove any decorators from

I'm unsure at this stage of the whole ramification that changes might cause however.

I continued my investigation by comparing the transpiled JS used in Stencil with the output of stencil build

I noticed that both files were not using the same decorate utilities:

I noticed that stencil build do set imperatively the experimentalDecorator options to true, and that even if it is set to false by the user, it is ignored.

const optionsToExtend = getTsOptionsToExtend(config);

export const getTsOptionsToExtend = (config: d.ValidatedConfig): ts.CompilerOptions => {
const tsOptions: ts.CompilerOptions = {
experimentalDecorators: true,
// if the `DIST_TYPES` output target is present then we'd like to emit
// declaration files
declaration: config.outputTargets.some(isOutputTargetDistTypes),
module: ts.ModuleKind.ESNext,
moduleResolution: ts.ModuleResolutionKind.NodeJs,
noEmitOnError: false,
outDir: config.cacheDir || config.sys.tmpDirSync(),
sourceMap: config.sourceMap,
inlineSources: config.sourceMap,
};
return tsOptions;
};

The transpile-module derives its tsconfig from

export const getTranspileConfig = (input: TranspileOptions): TranspileConfig => {
if (input.sys) {
, which does not do impose/set the experimentalDecorators option.

Therefore, I think for consistency, it'd be best for the transpile-options to set experiementalDecorators to true.
(patching it in the example of @Aadhisivam do make the test pass and the decorator behaving as expected (i.e. it logs the propName)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants