Skip to content
This repository has been archived by the owner on Sep 12, 2019. It is now read-only.

generalize dependency and config file matching to a common utility and add UI for multiple matching detectors and scripts #96

Merged
merged 4 commits into from
Apr 8, 2019

Conversation

DavidWells
Copy link
Contributor

Look for react scripts in dev and regular deps. https://netlify.slack.com/archives/CGW2Y6XPH/p1554664767295400

https://drive.google.com/a/netlify.com/file/d/1HsmHeJN4fCXFPz_0PeIpBkOYRtSAgMC5/view?usp=drivesdk

This will likely effect other detectors depending on how users setup their dependancies

@DavidWells
Copy link
Contributor Author

DavidWells commented Apr 7, 2019

We will probably want to do deeper checks like:

  1. Does project have dependancy XYZ, devDependancy XYZ, or peerDependancy XYZ
  2. Check for common config files of a project to match
  3. Does npm start or npm dev, or npm run build contain a reference to dependency. E.g. Does the build command from netlify.toml/npm scripts contain react-scripts (or any shorthand command of the binary)

This will result in more conclusive matches.

Its possible that more than 1 detector will match. In this case, we should prompt the user to show them a prompt to choose from the matches and then set the type of detector in the [dev] block or console a message for them to do so

@swyxio
Copy link
Contributor

swyxio commented Apr 7, 2019

peerDeps doesnt make that much sense. they're for plugins. npm/npm#1400

yes the possibility of multiple matches always made me uncomfortable. i like this idea a lot.

@swyxio swyxio changed the title Update cra.js generalize dependency and config file matching to a common utility and assign specificity score Apr 7, 2019
@swyxio swyxio changed the title generalize dependency and config file matching to a common utility and assign specificity score generalize dependency and config file matching to a common utility and add UI for multiple matching detectors Apr 8, 2019
@swyxio swyxio changed the title generalize dependency and config file matching to a common utility and add UI for multiple matching detectors generalize dependency and config file matching to a common utility and add UI for multiple matching detectors and scripts Apr 8, 2019
@swyxio
Copy link
Contributor

swyxio commented Apr 8, 2019

i changed my mind regarding using a specificity score. it feels like a premature optimization for now. so now we just have this UI to help disambiguate

image

i have chosen not to implement code to persist selections to netlify.toml dev block as i dont have time but i think that is a natural next step to this

@swyxio swyxio merged commit f749d38 into master Apr 8, 2019
@swyxio swyxio deleted the cra-detector-update branch April 8, 2019 01:36
@swyxio
Copy link
Contributor

swyxio commented Apr 8, 2019

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

Successfully merging this pull request may close these issues.

2 participants