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

🐛 [BUG]: Nx16 not supported #376

Closed
1 task done
Lonli-Lokli opened this issue May 12, 2023 · 11 comments · Fixed by #425
Closed
1 task done

🐛 [BUG]: Nx16 not supported #376

Lonli-Lokli opened this issue May 12, 2023 · 11 comments · Fixed by #425
Labels
scope: astro Related to @nxtensions/astro package type: bug Something isn't working

Comments

@Lonli-Lokli
Copy link

Lonli-Lokli commented May 12, 2023

Is there an existing issue for this?

  • I have searched the existing issues

What package does the issue affects?

@nxtensions/astro

Describe the bug

Nx16 not supported
https://nx.dev/recipes/other/rescope

Expected behavior

Nx16 supported

Steps to reproduce

npx create-nx-workspace@latest cv-dev --preset=empty
success


>  NX  Generating @nxtensions/astro:application

√ What frameworks and/or addons would you like to enable? · No items were selected

 >  NX   Cannot find module '@nrwl/js'

   Require stack:
   - D:\github\cv\node_modules\@nxtensions\astro\src\generators\init\generator.js
   - D:\github\cv\node_modules\@nxtensions\astro\src\generators\application\generator.js
   - D:\github\cv\node_modules\nx\src\config\workspaces.js
   - D:\github\cv\node_modules\nx\src\config\configuration.js
   - D:\github\cv\node_modules\nx\src\utils\package-manager.js
   - D:\github\cv\node_modules\nx\bin\init-local.js
   - D:\github\cv\node_modules\nx\bin\nx.js
   Pass --verbose to see the stacktrace.

Link to minimal reproducible example

No response

Environment

NX Its time to update Nx 🎉

Your repository uses a higher version of Nx (16.1.4) than your global CLI version (15.9.2)
For more information, see https://nx.dev/more-concepts/global-nx

NX Report complete - copy this into the issue template

Node : 18.16.0
OS : win32 x64
npm : 9.5.1
Hasher : Native

nx (global) : 15.9.2
nx : 16.1.4
@nrwl/jest : 12.8.0
@nrwl/linter : 12.8.0
@nx/workspace : 16.1.4
@nrwl/cypress : 12.8.0
@nx/devkit : 16.1.4
@nrwl/react : 12.8.0
@nrwl/storybook : 12.8.0
@nrwl/tao : 16.1.4
@nrwl/web : 12.8.0
nx-cloud : 16.0.5

Community plugins:
@nxtensions/astro : 3.5.0

The following packages should match the installed version of nx
- @nrwl/jest@12.8.0
- @nrwl/linter@12.8.0
- @nrwl/cypress@12.8.0
- @nrwl/react@12.8.0
- @nrwl/storybook@12.8.0
- @nrwl/web@12.8.0

To fix this, run nx migrate nx@16.1.4

@Lonli-Lokli Lonli-Lokli added needs triage This issue still needs to be triaged by a core team member type: bug Something isn't working labels May 12, 2023
@Lonli-Lokli Lonli-Lokli changed the title 🐛 [BUG]: 🐛 [BUG]: Nx16 not supported May 12, 2023
@Lonli-Lokli
Copy link
Author

Also even with Nx 15.9.0 it's failing

npx nx g @nxtensions/astro:app website --verbose

>  NX  Generating @nxtensions/astro:application

√ What frameworks and/or addons would you like to enable? · No items were selected
Fetching prettier...

 >  NX   The "path" argument must be of type string. Received undefined


TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
    at new NodeError (node:internal/errors:399:5)
    at validateString (node:internal/validators:163:11)
    at join (node:path:429:7)
    at FsTree.normalize (D:\github\cv-dev\node_modules\nx\src\generators\tree.js:221:64)
    at FsTree.exists (D:\github\cv-dev\node_modules\nx\src\generators\tree.js:84:25)
    at Object.readJson (D:\github\node_modules\@nrwl\devkit\src\utils\json.js:13:15)
    at Object.getWorkspaceLayout (D:\github\node_modules\@nrwl\devkit\src\utils\get-workspace-layout.js:20:33)
    at normalizeOptions (D:\github\node_modules\@nrwl\cypress\src\generators\cypress-project\cypress-project.js:150:34)
    at D:\github\node_modules\@nrwl\cypress\src\generators\cypress-project\cypress-project.js:138:25
    at Generator.next (<anonymous>)

@leosvelperez leosvelperez added scope: astro Related to @nxtensions/astro package and removed needs triage This issue still needs to be triaged by a core team member labels Jun 4, 2023
@doug-shontz
Copy link

Any update on this? I too am looking to put an Astro project into my NX monorepo and we can't even npm i the astro extensions due to a restriction on not using legacy peer deps. Would be glad to work on some sort of PR if I knew where to start looking!

@bigmistqke
Copy link

The lack of response from nx on this issue makes me reconsider using nx

@tobysmith568
Copy link

tobysmith568 commented Jul 11, 2023

Hey @leosvelperez :)

Forgive me for not knowing, is there more to this issue other than updating the nx packages and then running the nx migrations?

Like @doug-shontz above, I'm also happy to dedicate some time to this if I (or Doug) can get some pointers, Thanks!

@Brian-McBride
Copy link

The lack of response from nx on this issue makes me reconsider using nx

I don't think this extension is a Nx.dev official plugin. Sure, reconsider - but I wouldn't be pointing fingers at the nx team.
Keep i mind that community open source like this is primarily supported when the person who made it uses it often. Or the community adds to it.

My guess is that there are just not enough Astro users to help maintain this, and the author is using some other framework daily.

Astro is also growing fast too. It would be interesting if the Astro team would help maintain a plugin. I will say from my experience that building component libs that export Astro pages/components didn't work super well. IDE issues and typescript typing issues. :(

@bigmistqke
Copy link

I don't think this extension is a Nx.dev official plugin.

I thought it was, thanks for the info.

@leosvelperez
Copy link
Member

Hey all!

Apologies for the lack of updates here.

As others have mentioned already, this is an OSS project that I basically maintain in my free time. I don't use Astro in my day-to-day work and it's not an official Nx plugin. We all have lives and lately, I've been through some stuff that has left me with little to no time or strength to dedicate to the project. I'll eventually be back and have more time, but unfortunately, I can't promise anything yet.

I'd be more than happy to accept some contributions. If someone wants to tackle adding support for Nx 16, here are a few things to keep in mind:

  • The aim is to support multiple versions of Nx to give some breathing room to those who can't update Nx for whatever reason. So far, I've been trying to support from v14 onwards, with the idea of always supporting the latest version plus the previous major version.
  • So, when supporting Nx v16, we could drop support for Nx v14.
  • It also means that we need to handle importing either from @nrwl/* or @nx/*. The former is what exists in Nx v15, while the latter is what's available in Nx 16.
  • Any change or API consumed from Nx needs to be available in both versions.

So far, I've kept an eye on that by performing some manual testing. I've considered adding explicit E2E tests for that, but I haven't had the time to do so. It's something to do later.

@Brian-McBride
Copy link

The aim is to support multiple versions of Nx to give some breathing room to those who can't update Nx for whatever reason. So far, I've been trying to support from v14 onwards, with the idea of always supporting the latest version plus the previous major version.

Honestly, isn't that what semvr is for?
If someone wants to stay on Nx v14, then they could use your current pinned version of this plugin. They will have to do the same for all the other plugins as well.

I would just suggest going for a major version update. While it is jumping way ahead and doesn't make 100% sense (semvr wise) sometimes I think pluings should just match their major versions with the target platform versions. So if Nx is at 16, this plugin releases its major as 16 then minors at whatever.

Hoenstly though, better to just cut support of older Nx versions and let people pin what they need.

@Brian-McBride
Copy link

@Lonli-Lokli The path error could be related to this:

withastro/astro#6918

@donholly
Copy link

FWIW I was able to fix this by doing the following:

yarn add -D @nrwl/[missing_dependency]

in my case, I needed @nrwl/linter and @nrwl/cypress

Logs:

❯ npx nx g @nxtensions/astro:app website --verbose
>  NX  Generating @nxtensions/astro:application
✔ What frameworks and/or addons would you like to enable? · No items were selected
 >  NX   Cannot find module '@nrwl/linter'

then I ran:

yarn add -D @nrwl/linter

and then npx nx g @nxtensions/astro:app website --verbose again and it worked.

I'm on "nx": "16.5.2"

This should still be fixed though - @nrwl/ dependencies are going to be removed starting in NX version 17+

Source: https://nx.dev/recipes/other/rescope#@nrwl-scope-end-of-life

@leosvelperez
Copy link
Member

The aim is to support multiple versions of Nx to give some breathing room to those who can't update Nx for whatever reason. So far, I've been trying to support from v14 onwards, with the idea of always supporting the latest version plus the previous major version.

Honestly, isn't that what semvr is for?

No really. The decision to support (or not) multiple versions has nothing to do with Semver. Semver doesn't dictate whether a plugin should support a single version of any tool or not.

The point with supporting multiple versions is that regardless of the version of Nx you're using, you'd be able to use and benefit from all the new features or bug fixes from the latest version of this plugin. That was the goal. When supporting a single version, that can still be achieved but requires back-porting features and bug fixes to the lower versions of the plugins.

So far, I preferred the former to avoid having to back-port things between versions, but given the still low usage of the plugin and in order to make upgrades easier, I'm going to go with supporting a single Nx version moving forward.

So, I'll make the changes to support Nx 16, and as you suggested, I'm going to align the plugin version to the Nx version so it's easy to know which version to use based on your Nx version. I hope to have enough time today to make the changes and publish the release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: astro Related to @nxtensions/astro package type: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants