-
Notifications
You must be signed in to change notification settings - Fork 73
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
feat(prepare): allow prepare for layers #309
Conversation
Thanks for PR but i think it seems a very costly operation to run prepare on every layer as a workaround... If finally we need this, we can introduce a new meta for layers that needs prepare. It also seems an undesired behavior that layers are not in |
I see your point, but I can't say that this would be significantly costly for a simple Nuxt project, it would only be costlier in more complex projects with a lot of layers and rightfully so (in my opinion) as the bug is also critical. Since layers is a Nuxt concept, I don't think this will be a |
Layers being cloned outside of current dir can be solvalabe by simply ensuring there is an empty |
Sorry, I'm not sure I understand! |
I believe that would be for layers with prefix |
Ah, I see what you mean. Yes for package manager-managed layers it is different. (still strange tough as far I remember pmpm workspaces also makes a symlink in subdir) Anyway, I think running prepare as an opt-in feature for layers makes sense still. What we can do is to set something like (1) (I guess we need to finally discuss about convention for key name internally but this is general approach I am thinking about) |
Opt-in makes sense but not the most easy in my thinking - layer maintainers may need to add this in and they may forget (but someone will definitely point out). Or I'm thinking if we add a |
Here I understand that despite the symlink, the resolved |
extends
/_layers
Added in the |
@pi0 what do you think? Hoping for this to be addressed in the next minor release. (tailwind module docs deployments are also blocked by this) |
So far we have the issue as soon as we use pnpm workspace + a layer that uses types, the Tailwind Docs is a good example to reproduce (try to run Oneworkaround I found was to update the root {
"extends": "./docs/.nuxt/tsconfig.json"
} But this is not a correct solution as we break the module typings. Other workaround is to disable pnpm workspaces. I do find the solution quite nice since it only prepare the layer if the rootDir is different. |
We had been talking with @danielroe about possibly doing this directly in Nuxt core to smartly generate types for layers as well (when not published or available) also this logic is mostly relevant to the Nuxt core to control (this way we don't need to progressively discover layers like we do in this PR) |
That would be awesome indeed. Do you think of a kind of way to publish the types if possible? I was also thinking that the But I will let you with @danielroe think about the best integration 😊 |
In case of using npm publish or git tags, we might directly run
Probably this should also directly go to the Nuxt core to have full control but I fully agree. We should try to adopt layer meta for the future it can come in handy. |
If I'm not wrong, |
yes indeed, we need to specifically generate types that can be portable. For local utils and auto-imports, it should be doable I guess at least. I guess the next step would be to support generating types for layers in core |
Publishing types separately is ideal imo, and unbuild does that for us, but this issue arises from usage of TS in Vue components that uses |
If I'm right in understanding, https://github.com/nuxt/nuxt/blob/main/packages/kit/src/template.ts#L255 here we can add await Promise.all(
nuxt.options._layers.map(
layer => fsp.writeFile(resolve(layer.cwd, 'tsconfig.json'), GeneratedBy + `\n` + JSON.stringify({ extends: tsConfigPath }, null, 2))
)
) I'm thinking that this being added to core will still make this expensive, won't it? Of course, layer metadata options is another check that can narrow this a fair bit. |
We actually have to recursively call |
Hi dear @ineshbose coming back to this issue, i think we can move it to nuxt core. My understanding is that repro (https://github.com/nuxt/ui-pro/issues/118) is already resolved somehow else. |
A layer may make use of a module and then use imports from it through alias. The main project that uses this layer should also install this module, but even when that's the case, these imports aren't resolved unless it is in a corresponding
tsconfig.json
, but that may not be the case (in a monorepo especially, where your Nuxt app is inpackages/app
, and a layer may be in../node_modules
). We need to ship layers withtsconfig.json
most likely, but will still need to generate paths resolved accordingly.Refer https://github.com/nuxt/ui-pro/issues/118