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

New rule 'no unsupported Node APIs in SSR-able components' #167

Conversation

Shinoni
Copy link
Contributor

@Shinoni Shinoni commented Oct 16, 2024

No description provided.

Comment on lines 12 to 15
if (import.meta.env.SSR) {
// Risky Node API call within SSR context
fs.writeFileSync('file.txt', 'data');
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (import.meta.env.SSR) {
// Risky Node API call within SSR context
fs.writeFileSync('file.txt', 'data');
}
// Unsupported Node API call
fs.writeFileSync('file.txt', 'data');

Comment on lines 20 to 29
```js
if (import.meta.env.SSR) {
try {
// Safe guarded call
require('something');
} catch (e) {
// Handle error
}
}
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
```js
if (import.meta.env.SSR) {
try {
// Safe guarded call
require('something');
} catch (e) {
// Handle error
}
}
```
```js
// Do not use Node APIs

@Shinoni Shinoni changed the title New rule 'no-unguarded risky Node APIs in SSR-able components' New rule 'no unsupported Node APIs in SSR-able components' Oct 17, 2024
Shinoni and others added 2 commits October 17, 2024 14:42
Co-authored-by: Laura <49494194+lpomerleau@users.noreply.github.com>
@Shinoni Shinoni requested a review from lpomerleau October 17, 2024 23:36
README.md Outdated
| [lwc/no-template-children](./docs/rules/no-template-children.md) | prevent accessing the immediate children of this.template | |
| [lwc/no-leaky-event-listeners](./docs/rules/no-leaky-event-listeners.md) | prevent event listeners from leaking memory | |
| [lwc/prefer-custom-event](./docs/rules/prefer-custom-event.md) | suggest usage of `CustomEvent` over `Event` constructor | |
| [lwc/no-unsupported-node-api-in-ssrable-components](./docs/rules/no-unsupported-node-api-in-ssrable-components.md) | disallow unsupported Node API calls in SSR-able components | |
Copy link
Contributor

@lpomerleau lpomerleau Oct 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| [lwc/no-unsupported-node-api-in-ssrable-components](./docs/rules/no-unsupported-node-api-in-ssrable-components.md) | disallow unsupported Node API calls in SSR-able components | |
| [lwc/ssr/no-unsupported-node-api](./docs/rules/no-unsupported-node-api-in-ssrable-components.md) | disallow unsupported Node API calls in SSR-able components | |

Feedback from @wjhsf

In the eslint plugin, there’s currently no consistency in the SSR rule names:

  • lwc/no-restricted-browser-globals-during-ssr
  • lwc/no-unsupported-ssr-properties
  • lwc/no-node-env-in-ssr
  • lwc/no-unsuppoted-node-api-in-ssrable-components
  • lwc/no-unguarded-async-event-in-ssr
  • lwc/no-form-factor-in-ssrable-components
  • lwc/no-static-imports-of-user-specific-scoped-modules-in-ssrable-components

To have a more consistent naming convention (and terser names), please change all of them to the format lwc/ssr/*.

For rule and doc files names, Either "ssr/" or "ssr-" is probably fine. It depends on what eslint complains about, since it likes things to match the rule names.

For the few older ones that have already been published, we could either do a rename and release a major, or we could keep the old ones as aliases, but mark them as deprecated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I'll rename them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I renamed all references to the rule by prepending 'ssr-'.

@@ -0,0 +1,28 @@
# Disallow Node API Calls in SSR Context (`lwc/no-unsupported-node-api-in-ssrable-components`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Disallow Node API Calls in SSR Context (`lwc/no-unsupported-node-api-in-ssrable-components`)
# Disallow Node API Calls in SSR Context (`lwc/ssr/no-unsupported-node-api`)

Copy link
Contributor Author

@Shinoni Shinoni Oct 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lpomerleau , we can't modify the path to a rule without altering the framework, but we can rename a rule by adding the prefix 'ssr-'.

@Shinoni Shinoni merged commit d05b230 into salesforce:master Oct 18, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants