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

Property key of ClassMethod expected node to be of a type ["Identifier","StringLiteral","NumericLiteral"] but instead got undefined #1030

Closed
ntucker opened this issue Jul 30, 2022 · 6 comments · Fixed by #1032
Labels
bug report 🦗 Issue is probably a bug, but it needs to be checked bundler: webpack 📦 Issue is related to webpack bundler needs: complete repro 🖥️ Issue need to have complete repro provided

Comments

@ntucker
Copy link

ntucker commented Jul 30, 2022

Environment

  • Linaria version: 4.1.1
  • Bundler (+ version): webpack 5.74.0
  • Node.js version: 18.6
  • OS: Ubuntu 20.04
  • Node 18.6

Description

/home/ntucker/src/rest-hooks/node_modules/@babel/types/lib/definitions/utils.js:131
    throw new TypeError(`Property ${key} of ${node.type} expected node to be of a type ${JSON.stringify(types)} but instead got ${JSON.stringify(val == null ? void 0 : val.type)}`);
          ^

TypeError: /home/ntucker/src/rest-hooks/packages/rest/lib/BaseResource.js: Property key of ClassMethod expected node to be of a type ["Identifier","StringLiteral","NumericLiteral"] but instead got undefined
    at validate (/home/ntucker/src/rest-hooks/node_modules/@babel/types/lib/definitions/utils.js:131:11)
    at /home/ntucker/src/rest-hooks/node_modules/@babel/types/lib/definitions/core.js:1308:9
    at Object.validate (/home/ntucker/src/rest-hooks/node_modules/@babel/types/lib/definitions/utils.js:228:7)
    at validateField (/home/ntucker/src/rest-hooks/node_modules/@babel/types/lib/validators/validate.js:24:9)
    at validate (/home/ntucker/src/rest-hooks/node_modules/@babel/types/lib/validators/validate.js:17:3)
    at NodePath._replaceWith (/home/ntucker/src/rest-hooks/node_modules/@babel/traverse/lib/path/replacement.js:169:5)
    at NodePath._remove (/home/ntucker/src/rest-hooks/node_modules/@babel/traverse/lib/path/removal.js:59:10)
    at NodePath.remove (/home/ntucker/src/rest-hooks/node_modules/@babel/traverse/lib/path/removal.js:38:8)
    at /home/ntucker/src/rest-hooks/node_modules/@linaria/utils/lib/scopeHelpers.js:296:42
    at mutate (/home/ntucker/src/rest-hooks/node_modules/@linaria/utils/lib/scopeHelpers.js:327:19) {
  code: 'BABEL_TRANSFORM_ERROR'
}

This is 'property key':

  static get key() {
    /* istanbul ignore else */
    if (process.env.NODE_ENV !== 'production') {
      if (this.urlRoot === undefined) {
        throw new Error(`urlRoot is not defined for Resource "${this.name}"

  Resources require a 'static urlRoot' or 'static get key()' defined.
  (See https://resthooks.io/docs/api/resource#static-urlroot-string)
`);
      }
    }

    return this.urlRoot;
  }

Note that rest/lib/BaseResource.js is 100% node compatible ESM. All imports use fully qualified extensions, and the package.json has "type": "module". So there is actually no reason one would need to run babel on it.

Perhaps there are three situations

  1. make babel not explode on getters
  2. make the shaker possibly ignore 'correct' ESM that can be interpreted by node. This could maybe be detected by use of 'exports' field rather than 'module' from a package.json.
  3. Nothing in the 'Resource' class is used in a linaria block...so perhaps that is also shakable?

Reproducible Demo

reactive/data-client#2117

  1. git clone git@github.com:coinbase/rest-hooks.git
  2. cd rest-hooks
  3. corepack enable
  4. git checkout d9d18df130cf4592e03af4f39edc57e695f876c0
  5. nvm use (node 18 is required)
  6. yarn install
  7. yarn build
  8. cd examples/todo-app
  9. yarn start
@ntucker ntucker added bug report 🦗 Issue is probably a bug, but it needs to be checked needs: complete repro 🖥️ Issue need to have complete repro provided needs: triage 🏷 Issue needs to be checked and prioritized labels Jul 30, 2022
@github-actions github-actions bot added bundler: webpack 📦 Issue is related to webpack bundler and removed needs: triage 🏷 Issue needs to be checked and prioritized labels Jul 30, 2022
@Anber
Copy link
Collaborator

Anber commented Jul 30, 2022

We cannot just ignore those files because we have to tree-shake them.
I'll fix this expected node to be of a type until Monday.

@ntucker
Copy link
Author

ntucker commented Jul 30, 2022

I could be wrong as I don't understand all the internals, but https://github.com/coinbase/rest-hooks/blob/renovate/major-webpack/examples/todo-app/src/pages/Home/TodoListItem.tsx#L32 is not using any imported symbols beyond linaria, so I was thinking perhaps those imports don't have to be traversed?

(https://github.com/coinbase/rest-hooks/blob/renovate/major-webpack/examples/todo-app/src/pages/Home/TodoListItem.tsx#L32 is the start of the chain that gets to BaseResource.js)

@Anber
Copy link
Collaborator

Anber commented Jul 31, 2022

Hi @ntucker
I can't reproduce it…

This is 'property key'
It doesn't have to be that specific ClassMethod. Property key in the error means, that in some of ClassMethods Linaria tries to remove key property.

I've made a fix but it would be cool if you can help me to reproduce the error, so I can add a test case.

@ntucker
Copy link
Author

ntucker commented Aug 1, 2022

I'm pretty sure it is that specific key, because Property ${key} of ${node.type} expected turns into "Property key of ClassMethod expected". Therefore the variable key is "key" and node.type === "ClassMethod".

Did you try reproducing using the instructions or did you just try extracting the part I thought was related?

@Anber
Copy link
Collaborator

Anber commented Aug 1, 2022

Therefore the variable key is "key"

The variable "key" here is not the value, but the name of the member of ClassMethod. The error means, that Linaria tries to replace the key in some object with undefined, instead of expected identifier, string or number. It is incorrect behaviour in any case and Linaria should delete the whole property if either key or body were deleted, but I just cannot understand why it tries to replace key in that specific case. It may be some unexpected behaviour in the shaker, so it would be cool if I can reproduce it.

Did you try reproducing using the instructions

I tried the instruction and everything was ok.

@ntucker
Copy link
Author

ntucker commented Aug 1, 2022

Good news - the new version does fix this issue.

I can think of two possible reasons you can't reproduce:

  1. Do you have corepack enabled? Not using it will mean packages won't install correctly (I added corepack enable to instructions above)
  2. If you already have the rest-hooks repo checked out; the build cache will avoid triggering this condition:
  3. yarn build:clean
  4. after moving to examples/todo-app: rm -rf node_modules/.cache

If these aren't the case then I guess it's probably not worth figuring out - since the issue has been fixed. Thanks for the hard work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report 🦗 Issue is probably a bug, but it needs to be checked bundler: webpack 📦 Issue is related to webpack bundler needs: complete repro 🖥️ Issue need to have complete repro provided
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants