Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

feat: migrate from webpack to vite #819

Merged
merged 8 commits into from
Oct 18, 2022
Merged

Conversation

ericapisani
Copy link
Contributor

@ericapisani ericapisani commented Oct 7, 2022

Summary

Replaces webpack with vite to build.

Is far easier to configure and maintain.

Testing

cli

  1. Pull this branch, increment the version in package.json of the package, by running npm version major (major bacause Node.js 12 has been dropped in favour of Node.js 14) and run npm pack
  2. In the build-info project, run npm install and then run npm install <path to the .tgz>, e.g. npm install ../framework-info/netlify-framework-info-10.0.0.tgz
  3. Run npm link from the root of the build-info repository
  4. Run build-info <project path> against a test project
  5. If working correctly, frameworks should be detected (such as a 'Next.js' site)

UI

  1. Do the same as step 1. for the cli
  2. In the netlify-react-ui project, run yarn install and then run yarn add <path to the .tgz>, e.g. yarn add ../framework-info/netlify-framework-info-10.0.0.tgz
  3. follow the steps to start the UI locally
  4. Start the flow of importing a site from a git repo
  5. If working correctly, the correct framework should be detected (such as a 'Next.js' site)

For us to review and ship your PR efficiently, please perform the following steps:

  • Open a bug/issue before writing your code 🧑‍💻. This
    ensures we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a
    typo or something that`s on fire 🔥 (e.g. incident related), you can skip this step.
  • Read the contribution guidelines 📖. This ensures your code follows our style guide and
    passes our tests.
  • Update or add tests (if any source code was changed or added) 🧪
  • Update or add documentation (if features were changed or added) 📝
  • Make sure the status checks below are successful ✅

A picture of a cute animal (not mandatory, but encouraged)

IMG_20221001_175611_852

@github-actions github-actions bot added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Oct 7, 2022
@ericapisani ericapisani self-assigned this Oct 7, 2022
@netlify
Copy link

netlify bot commented Oct 7, 2022

Deploy Preview for framework-info ready!

Name Link
🔨 Latest commit 877718b
🔍 Latest deploy log https://app.netlify.com/sites/framework-info/deploys/634811a9d358d50008de7f1a
😎 Deploy Preview https://deploy-preview-819--framework-info.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@ericapisani ericapisani marked this pull request as ready for review October 7, 2022 18:45
{
"name": "test",
"version": "1.0.0"
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This needed to be added because otherwise the logic in framework-info would detect the root project's package.json. This ensures that we're truly testing when no framework is detected in a project

test/detect.js Outdated
@@ -31,7 +31,7 @@ test('Should ignore if matching any framework.excludedNpmDependencies', async (t

test('Should detect config files', async (t) => {
const frameworks = await getFrameworks('config_files')
t.is(frameworks.length, 1)
t.is(frameworks.length, 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.

Because vite exists within the project, framework-info is now picking that up as well as the framework for which these were originally checking for

Copy link
Contributor

Choose a reason for hiding this comment

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

Adding a package.json in the test/fixtures/config_files folder like in test/fixtures/empty fixes this, so it only picks up the one framework, middleman.

e.g. package.json

{
  "name": "test",
  "version": "1.0.0"
}

So this can stay as

Suggested change
t.is(frameworks.length, 2)
t.is(frameworks.length, 1)

sarahetter
sarahetter previously approved these changes Oct 11, 2022
Copy link

@sarahetter sarahetter left a comment

Choose a reason for hiding this comment

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

LGTM!

test/detect.js Outdated
@@ -31,7 +31,7 @@ test('Should ignore if matching any framework.excludedNpmDependencies', async (t

test('Should detect config files', async (t) => {
const frameworks = await getFrameworks('config_files')
t.is(frameworks.length, 1)
t.is(frameworks.length, 2)
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding a package.json in the test/fixtures/config_files folder like in test/fixtures/empty fixes this, so it only picks up the one framework, middleman.

e.g. package.json

{
  "name": "test",
  "version": "1.0.0"
}

So this can stay as

Suggested change
t.is(frameworks.length, 2)
t.is(frameworks.length, 1)

@nickytonline
Copy link
Contributor

I ran build-info ../../oss/iamdeveloper.com for my blog (Eleventy) and it gave the expected output.

{
  "frameworks": [
    {
      "id": "eleventy",
      "name": "Eleventy",
      "category": "static_site_generator",
      "dev": {
        "commands": ["npm run serve"],
        "port": 8080,
        "pollingStrategies": [
          {
            "name": "TCP"
          },
          {
            "name": "HTTP"
          }
        ]
      },
      "build": {
        "commands": ["eleventy"],
        "directory": "_site"
      },
      "env": {},
      "logo": {
        "default": "https://framework-info.netlify.app/logos/eleventy/default.svg",
        "light": "https://framework-info.netlify.app/logos/eleventy/default.svg",
        "dark": "https://framework-info.netlify.app/logos/eleventy/default.svg"
      },
      "plugins": []
    }
  ]
}

Copy link
Contributor

@nickytonline nickytonline left a comment

Choose a reason for hiding this comment

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

Thanks for making the changes! 🚀

@ericapisani ericapisani added automerge type: feature code contributing to the implementation of a feature and/or user facing functionality and removed type: feature code contributing to the implementation of a feature and/or user facing functionality labels Oct 13, 2022
@MarcL MarcL removed the type: feature code contributing to the implementation of a feature and/or user facing functionality label Oct 14, 2022
@MarcL MarcL added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Oct 14, 2022
@ericapisani ericapisani merged commit 13430d9 into main Oct 18, 2022
@ericapisani ericapisani deleted the migrate-from-wp-to-vite branch October 18, 2022 14:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
automerge type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants