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

Import mdn/browser-compat-data. Fix #102 #105

Merged
merged 6 commits into from
Jun 18, 2018
Merged

Import mdn/browser-compat-data. Fix #102 #105

merged 6 commits into from
Jun 18, 2018

Conversation

octref
Copy link
Contributor

@octref octref commented Jun 11, 2018

  • Add mdn/browser-compat-data as a dev dependency
  • Use bcd data whenever it's available

Cosmetics:

  • Made all js scripts under /build the same style
  • Switch to const/let from var


var { buildPropertiesWithMDNData } = require('./mdn-data-importer')
const { buildPropertiesWithMDNData } = require('./mdn-data-importer')
const { addBCPDataToProperties } = require('./mdn-browser-compat-data-importer')
Copy link
Contributor

Choose a reason for hiding this comment

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

BCP?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah it should be BCD. Thanks.

* }
* } => "FF6"
*/
function supportToShortCompatString(support, browserAbbrev) {
Copy link
Contributor

@connorshea connorshea Jun 11, 2018

Choose a reason for hiding this comment

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

This doesn't completely handle cases with more complex data, for example when a feature is behind a flag this would think it's supported:

{
  "support": {
    "chrome": [
      {
        "version_added": "67",
        "flags": [
          {
            "type": "preference",
            "name": "Enable Experimental Web Platform Features",
            "value_to_set": "true"
          }
        ]
      }
    ]
  }
}

Or when a feature was removed:

{
  "support": {
    "firefox": [
      {
        "version_added": "51",
        "version_removed": "54"
      }
    ]
  }
}

Maybe acceptable for an MVP, not sure :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@connorshea I opened #107 for that.

function addBrowserCompatDataToProperties(atdirectives, pseudoclasses, pseudoelements, properties) {
atdirectives.forEach(item => {
if (bcd.css['at-rules'][item.name.slice(1)]) {
const matchingBCPItem = bcd.css['at-rules'][item.name.slice(1)]
Copy link
Contributor

Choose a reason for hiding this comment

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

there's also a bunch of BCP in this file :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@connorshea You are right. Originally wanted to call it browser-compat-property.

@octref octref changed the title [WIP] Import mdn/browser-compat-data. Fix #102 Import mdn/browser-compat-data. Fix #102 Jun 18, 2018
@octref
Copy link
Contributor Author

octref commented Jun 18, 2018

@aeschli I updated the PR to use all for properties supported in all browsers, so they don't clutter the output.

One thing I left out is dropping Opera. browser-compat-data also has data for mobile browser supports such as Safari iOS, FF Android, etc. However I don't want to include all browser compat data to clutter the completion item's doc.

@octref octref merged commit aed9968 into master Jun 18, 2018
@octref octref deleted the mdn-compat branch June 18, 2018 22:44
@aeschli aeschli added this to the June 2018 milestone Jun 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants