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

Remove support for process.server/process.client #25323

Open
3 of 4 tasks
manniL opened this issue Jan 19, 2024 · 13 comments
Open
3 of 4 tasks

Remove support for process.server/process.client #25323

manniL opened this issue Jan 19, 2024 · 13 comments

Comments

@manniL
Copy link
Member

manniL commented Jan 19, 2024

Describe the feature

In v4 v5, we could remove support for process.server/process.client in place of import.meta.server/import.meta.client (and similar). We could even help with a codemod.

Impact might be modules providing components using it 🤔

Additional information

  • Would you be willing to help implement this feature?
  • Could this feature be implemented as a module?

Final checks

@Hebilicious
Copy link
Member

I absolutely agree it makes sense to not have to rely on these. I like import.meta.*, but I also worry that there's no standard and that library authors are starting to over-use import.meta.
Relevant: #21855

@bernhardberger
Copy link
Contributor

bernhardberger commented Jan 21, 2024

As someone that's on the outside of core things in Nuxt but whose target audience is Enterprise customers the following consideration is scaring me a bit:

Impact might be modules providing components using it 🤔

V2 -> V3 already almost broke the ecosystem (granted, upstream Vue also had a lot to do with this also) and I'm scared that too much/many low-level breaking changes from one version to another (that could potentially break a lot of deps in bigger projects) could scare away even more potential enterprise users / devs.

I'd really like to see soft deprecation first and finally removing support for ground breaking stuff only the major version after to give devs time to sort those kind of changes out in a reasonable timeframe without breaking things "overnight".

I know that this might not be the place to discuss this, but it raises a concern for me. I've already seen some scare in the community because of talk of yearly major releases and I urge you to handle major breaking changes with care between yearly major version releases. Please give the ecosystem time to get stable and stay stable.

Long Term Support, plan-ability and upgradeability are one of the major reasons for success of frameworks like Symfony or Laravel in the enterprise world.

@manniL
Copy link
Member Author

manniL commented Jan 21, 2024

Thanks for bringing up your concerns @bernhardberger! I can ensure you, there will be nothing as drastic as V2 -> V3 because lots of moving pieces changed. With V3 -> V4 that won't be the case at all (No Vue major, no migration to Vite, no Nuxt TS rewrite, No new server engine etc. etc.)

I'd really like to see soft deprecation first and finally removing support for ground breaking stuff only the major version after to give devs time to sort those kind of changes out in a reasonable timeframe without breaking things "overnight".

This is usually the way it goes 👍

Long Term Support, plan-ability and upgradeability are one of the major reasons for success of frameworks like Symfony or Laravel in the enterprise world.

Absolutely! I fully agree. We also discussed how we can make the next major migration as friction-free as possible (think of e.g. codemods and, as it happened already for minors, PRs to open-source modules) and will evaluate each suggestion, as this one, carefully.

@bernhardberger
Copy link
Contributor

Just to be clear on another part to this ticket: I like proposal in general and I especially like the discussion in #21855 - a unified and clear to understand/read API would be welcome. Abstracting away the low level stuff would also leave room to adapt the implementation details down the road, like this ticket proposes, with less impact.

@daniluk4000
Copy link
Contributor

Adding to this. I've tried to get away from process.server to import.meta.server and got error Cannot use 'import.meta' outside a module. I believe that removing this will also inproduce inconsistence for shared composables that use process.server/process.client.

@pi0
Copy link
Member

pi0 commented Jan 22, 2024

@daniluk4000 (or anyone else) if you are facing issues like Cannot use import.* outside a module, please share a reproduction. If any issues in tool chain that lack its support (for example older jiti versions didn't support it) we shall fix until making this change.

Obviously, (as for any other open namespace), library authors and users should be careful to not abuse import.meta namespace. Vendor/Framework prefixing is always best idea and for built-in ones, we try to communicated across ecosystems to make sure they won't conflict (it is no different than process* namespace that is only made for Node.js but now we are wrongly using for other runtimes via shims and polyfills)

@daniluk4000
Copy link
Contributor

daniluk4000 commented Jan 23, 2024

@pi0 it's easy.

export default defineNuxtModule({
    async setup(_, nuxt: Nuxt) {
        console.log(import.meta.server);
    }
})

Done.

image

Of course you can assume you're always on SSR inside module. Yet process.server could still be used inside some inner composable, process.server worked fine and import.meta does not work fine.

@pi0
Copy link
Member

pi0 commented Jan 23, 2024

Thanks for sharing @daniluk4000 yes indeed this is kind of stuff we need to resolve (in jiti).

The counterpoint is, we were always depending on Node.js process API as first class and mocking it with several ways for runtimes that don't have Node.js. The process is to remove this dependency and move to ESM native standards such as import.meta. And it needs to be supported before we can do this. Also value of process.server is undefiend in modules today. Node.js only is less strict about it and not making an error.

@Hebilicious
Copy link
Member

@daniluk4000 (or anyone else) if you are facing issues like Cannot use import.* outside a module, please share a reproduction. If any issues in tool chain that lack its support (for example older jiti versions didn't support it) we shall fix until making this change.

Obviously, (as for any other open namespace), library authors and users should be careful to not abuse import.meta namespace. Vendor/Framework prefixing is always best idea and for built-in ones, we try to communicated across ecosystems to make sure they won't conflict (it is no different than process* namespace that is only made for Node.js but now we are wrongly using for other runtimes via shims and polyfills)

Perhaps it would be a good idea to use import.meta.nuxt.* ? 🤔 It's more verbose but the intent is clear and we can put as many things as we want there without worrying about conflicts with other part of the toolchain.

@pi0
Copy link
Member

pi0 commented Jan 23, 2024

nuxt prefix makes sense! It also solves the ambiguity of which server we are in and also makes it easier to for example configure jiti especially handle with a prefix in AST.

@danielroe danielroe added 5.x and removed 4.x labels Mar 22, 2024
@danielroe
Copy link
Member

I absolutely agree we shouldn't remove process.* for v4. Instead we will soft-deprecate it and work to ensure ecosystem is migrated to backwards/forwards-compatible code.

kakkokari-gtyih added a commit to misskey-dev/misskey-hub-next that referenced this issue Mar 29, 2024
@rudolfbyker
Copy link
Contributor

One missing piece that I found when considering dropping process.server and process.client is that there is no nice and simple way (that I could find) to mock import.meta.server and import.meta.client in unit tests.

Examples

Here I'm using it in the template just to demonstrate, but in practice it's more likely to have statements like if (import.meta.*) in methods, e.g. when triggering some kind of side effect that has to be handled differently on the server side vs. the client side.

Example using import.meta.*

TestServerClient.vue:

<template>
  <p>{{ message }}</p>
</template>

<script setup lang="ts">
import { computed } from "vue";
const message = computed(() => {
  if (import.meta.server) {
    return "server";
  }
  if (import.meta.client) {
    return "client";
  }
  return "unknown";
});
</script>

TestServerClient.test.ts:

// @vitest-environment jsdom

import { mount, renderToString } from "@vue/test-utils";
import TestServerClient from "~/components/TestServerClient.vue";

describe("TestServerClient", () => {
  test("mount", async () => {
    const wrapper = await mount(TestServerClient);
    console.log(wrapper.html());
  });

  test("renderToString", async () => {
    const html = await renderToString(TestServerClient);
    console.log(html);
  });
});

Output:

<p>unknown</p>
<p>unknown</p>

Example using process.*

TestServerClient.vue:

<template>
  <p>{{ message }}</p>
</template>

<script setup lang="ts">
import { computed } from "vue";
const message = computed(() => {
  if (process.server) {
    return "server";
  }
  if (process.client) {
    return "client";
  }
  return "unknown";
});
</script>

TestServerClient.test.ts:

// @vitest-environment jsdom

import { mount, renderToString } from "@vue/test-utils";
import TestServerClient from "~/components/TestServerClient.vue";

describe("TestServerClient", () => {
  test("mount", async () => {
    process.client = true;
    process.server = false;
    const wrapper = await mount(TestServerClient);
    console.log(wrapper.html());
  });

  test("renderToString", async () => {
    process.client = false;
    process.server = true;
    const html = await renderToString(TestServerClient);
    console.log(html);
  });
});

Output:

<p>client</p>
<p>server</p>

@danielroe
Copy link
Member

@rudolfbyker It should work with @nuxt/test-utils in our runtime environment: https://nuxt.com/docs/getting-started/testing#unit-testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Has plan
Development

No branches or pull requests

7 participants