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.

Sign up for GitHub

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
Open

Update migration 11-12 guide #336

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b3968f7
Update migration 11-12 guide for changes in ES Lint and node cnofigs
piqusy Jul 25, 2024
f4c2ef4
Add a note on migration time
piqusy Jul 25, 2024
b8e9bc2
Update migration 11-12 guide - PR suggestions
piqusy Jul 26, 2024
ac17ceb
Fix boilperplate CLI example for manifest-cache
piqusy Jul 26, 2024
95dd3ab
Change the .swcrc example with a link to the libs file
piqusy Jul 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions website/docs/migrations/11-12.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,3 +218,46 @@ const projectConfig = {
},
};
```

### Update ES Lint config

**Migration time: ~10min.**

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
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 •
edited
Loading

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

import config from '@eightshift/frontend-libs/linters/eslint.config.mjs';

export default [{
ignores: ["public/**/*.js"],
}, ...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

```bash
npm install @eslint/js -D
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?

```json
Copy link
Member

@iruzevic iruzevic Jul 27, 2024 •
edited
Loading

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

{
"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:\
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

`/.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 😬