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

Consider removing NODE_ENV check from isServerSide #359

Open
zenorocha opened this issue Feb 17, 2018 · 1 comment
Open

Consider removing NODE_ENV check from isServerSide #359

zenorocha opened this issue Feb 17, 2018 · 1 comment

Comments

@zenorocha
Copy link
Contributor

zenorocha commented Feb 17, 2018

Today I was trying to test our components using Jest's Node.js environment instead of JSDom.

/**
 * @jest-environment node
 */

import Component from 'metal-component';
import Alert from '../src/Alert';

describe('Alert.node', () => {
  it('should not fail on the server side', () => {
    const alert = Component.renderToString(Alert);
    expect(alert).not.toBeNull();
  });
});

After an hour trying to understand why Jest was throwing client-side errors when it shouldn't, I realized it's because they set NODE_ENV equals test [1] and we use that same value on isServerSide [2].

I don't know what was the reason behind that, but I think that line shouldn't exist. Sure, I could just set a different value to that environment variable, but as we move into having components that also compile on the server, I think it's important to have a isServerSide that we can rely on. Otherwise, people would just create their own like what was done on WeDeploy's Console.

[1] jestjs/jest#3370
[2] https://github.com/metal/metal.js/blob/master/packages/metal/src/coreNamed.js#L307

@robframpton
Copy link

If we make this change now it will break a lot of tests for the other product teams that are using Jest, including Clay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants