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

Update migration 11-12 guide #336

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

piqusy
Copy link
Contributor

@piqusy piqusy commented Jul 25, 2024

Description

Add migration guide for changes in ES Lint and node version configs

Linked documentation PR

@piqusy piqusy requested a review from a team July 25, 2024 21:13
@piqusy piqusy self-assigned this Jul 25, 2024
Comment on lines 226 to 263
Depending on the project current state you might need to account for time spent fixing errors and warnings produced by the new ES Lint settings so the migration time may vary.

ES Lint configuration has been updated. To make sure everything works with the new version update the project node version to 20 or higher and update the ES Lint configuration. The steps are described below.

1. Delete the `.eslintrc.js` and `.eslintignore` files from the theme root.

2. Add `eslint.config.mjs` file to the theme root with the following content:
```js
import config from '@eightshift/frontend-libs/linters/eslint.config.mjs';

export default [{
ignores: ["public/**/*.js"],
}, ...config];
```

3. Install the required dependencies:
```bash
npm install @eslint/js -D
npm install @eslint/eslintrc -D
```

4. Add `.swcrc` to theme root with the following content:
```json
{
"jsc": {
"parser": {
"syntax": "ecmascript",
"jsx": true,
"decorators": false,
"dynamicImport": true
}
}
}
```

5. Make sure the project is using `node` version 20 or higher. If you are deploying with GitHub actions you will need to update to `node: [22.x]` version in deploy config files:\
`/.github/workflows/deploy.yml`\
`/.github/workflows/quality.yml`
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
Depending on the project current state you might need to account for time spent fixing errors and warnings produced by the new ES Lint settings so the migration time may vary.
ES Lint configuration has been updated. To make sure everything works with the new version update the project node version to 20 or higher and update the ES Lint configuration. The steps are described below.
1. Delete the `.eslintrc.js` and `.eslintignore` files from the theme root.
2. Add `eslint.config.mjs` file to the theme root with the following content:
```js
import config from '@eightshift/frontend-libs/linters/eslint.config.mjs';
export default [{
ignores: ["public/**/*.js"],
}, ...config];
```
3. Install the required dependencies:
```bash
npm install @eslint/js -D
npm install @eslint/eslintrc -D
```
4. Add `.swcrc` to theme root with the following content:
```json
{
"jsc": {
"parser": {
"syntax": "ecmascript",
"jsx": true,
"decorators": false,
"dynamicImport": true
}
}
}
```
5. Make sure the project is using `node` version 20 or higher. If you are deploying with GitHub actions you will need to update to `node: [22.x]` version in deploy config files:\
`/.github/workflows/deploy.yml`\
`/.github/workflows/quality.yml`
Depending on the state the project is in, you might need to allocate some time for fixing errors and warnings after updating the coding standards.
ESLint configuration has been updated in Frontend libs 12 and a new build config was added.
To implement the changes, follow these steps:
1. Delete the `.eslintrc.js` and `.eslintignore` files from the theme root (if they exist)
2. Add `eslint.config.mjs` file to the theme root with the following content:
```js
import config from '@eightshift/frontend-libs/linters/eslint.config.mjs';
export default [{
ignores: ["public/**/*.js"],
}, ...config];
  1. Install the required dependencies:
npm install @eslint/js -D
npm install @eslint/eslintrc -D
  1. Add a .swcrc file to theme root with the following content:
{
	"jsc": {
		"parser": {
			"syntax": "ecmascript",
			"jsx": true,
			"decorators": false,
			"dynamicImport": true
		}
	}
}
  1. Make sure the project is using NodeJS version 20 (or newer). If deploying with GitHub actions you will need to update to node: [22.x] version in deploy config files:
    /.github/workflows/deploy.yml
    /.github/workflows/quality.yml

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like suggestions with triple backticks break GH lol, but the suggestion preview is OK.

Also, you shouldn't need to do

  1. Install the required dependencies:
npm install @eslint/js -D
npm install @eslint/eslintrc -D

🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestions, you're right about the packages 😬

1. Delete the `.eslintrc.js` and `.eslintignore` files from the theme root.

2. Add `eslint.config.mjs` file to the theme root with the following content:
```js
Copy link
Member

Choose a reason for hiding this comment

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

don't add the exact code here as if something changes we need to update this guide. Add a link where they can see the code and copy it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you are referring to the Eightshift Frontend Libs config eslint.config.mjs file this example is different as it includes the ignore pattern which replaces the values from the .eslintignore deprecated file

Copy link
Member

Choose a reason for hiding this comment

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

well if this needs to be added in the config we should add it. @goranalkovic-infinum please check if this needs to be added

Copy link
Contributor

@goranalkovic-infinum goranalkovic-infinum Aug 1, 2024

Choose a reason for hiding this comment

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

@iruzevic We should replace the files in the default setup (standard version), but it should also stay in the guide since you won't have those files on existing projects. Same is valid for .swcrc

}, ...config];
```

3. Install the required dependencies:
Copy link
Member

Choose a reason for hiding this comment

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

you should not need to add this as this is part of the flibs

npm install @eslint/eslintrc -D
```

4. Add `.swcrc` to theme root with the following content:
Copy link
Member

Choose a reason for hiding this comment

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

the same as point 2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haven't found this, could you please provide the example URL?

}
```

5. Make sure the project is using `node` version 20 or higher. If you are deploying with GitHub actions you will need to update to `node: [22.x]` version in deploy config files:\
Copy link
Member

Choose a reason for hiding this comment

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

deploy process is not the part of the generic 8shift setup and any one can use this as is. You can leave the part for the node version but remove all the mentions of the deploy process

1. Delete the `.eslintrc.js` and `.eslintignore` files from the theme root.

2. Add `eslint.config.mjs` file to the theme root with the following content:
```js
Copy link
Member

Choose a reason for hiding this comment

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

well if this needs to be added in the config we should add it. @goranalkovic-infinum please check if this needs to be added

```

3. Add a `.swcrc` file to theme root with the following content:
```json
Copy link
Member

@iruzevic iruzevic Jul 27, 2024

Choose a reason for hiding this comment

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

remove this part and replace it with the link from the libs
https://github.com/infinum/eightshift-libs/blob/main/src/InitSetup/standard/theme/.swcrc

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