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

Improve dev tools to update browser variables list. #165

Closed
wants to merge 11 commits into from

Conversation

shapkarin
Copy link

@shapkarin shapkarin commented Jul 5, 2020

resolves #164 and also closes #166

@shapkarin shapkarin changed the title Update browser globals with "get-browser-globals.js" script Update browser globals with "get-browser-globals.js" script and fix the way we get the browsers variables Jul 5, 2020
globals.json Show resolved Hide resolved
@shapkarin shapkarin changed the title Update browser globals with "get-browser-globals.js" script and fix the way we get the browsers variables Update browser globals with "get-browser-globals.js" script and fix a little the way we get the browsers variables Jul 5, 2020
@shapkarin
Copy link
Author

shapkarin commented Jul 5, 2020

That's what I trying to do:

  • I forked jshint and eslint and added it github repo as devDependencies
  • next I plan to add it to the get-browser-globals.js script
  • in this way I can get the variables directly from the files
  • not sure if they will be imported as well to the result module but we can try some build trick to make that works out

globals.json Show resolved Hide resolved
@shapkarin shapkarin changed the title Update browser globals with "get-browser-globals.js" script and fix a little the way we get the browsers variables Add get-jshint-browser-globals.js and combine-browser-globals. [import jshints list directly and combine them with the get-browser-globals.js ones] Jul 6, 2020
@shapkarin
Copy link
Author

@sindresorhus
I start to automate that somehow. At least for some steps from update's readme.md

Usage

  • Run npm run get-jshint-browser.
  • Open an Incognito window in Chrome Canary and paste the above into the console.
  • You'll now have a new object in your clipboard.
  • Paste yours current clipboard to the field my_browser field at the root of the project.
  • run npm run combine-browser
  • Copy and paste the result from the browser_vars.json to the field browser at the globals.json

@shapkarin shapkarin changed the title Add get-jshint-browser-globals.js and combine-browser-globals. [import jshints list directly and combine them with the get-browser-globals.js ones] Add get-jshint-browser-globals.js and combine-browser-globals. [import jshints list directly and combine them with the get-browser-globals.js] Jul 6, 2020
@shapkarin
Copy link
Author

shapkarin commented Jul 6, 2020

@sindresorhus I hope it's a good one contribution and that can help me to make my package better.
Maybe even the jshint as well. I will be wait the review and some response.

@sindresorhus
Copy link
Owner

Can you describe how your PR is solving those referenced issues?

@sindresorhus
Copy link
Owner

We definitely don't want to fetch globals directly from JSHint. That readme reference just points out where the global originally came from.

@sindresorhus
Copy link
Owner

And there is already a similar PR open: #132 How is yours different?

@shapkarin
Copy link
Author

shapkarin commented Jul 12, 2020 via email

@shapkarin
Copy link
Author

shapkarin commented Jul 12, 2020 via email

@shapkarin
Copy link
Author

shapkarin commented Jul 13, 2020 via email

@shapkarin
Copy link
Author

shapkarin commented Jul 13, 2020 via email

@shapkarin
Copy link
Author

shapkarin commented Jul 13, 2020 via email

@shapkarin
Copy link
Author

shapkarin commented Jul 13, 2020 via email

@shapkarin
Copy link
Author

shapkarin commented Jul 13, 2020 via email

@shapkarin shapkarin changed the title Add get-jshint-browser-globals.js and combine-browser-globals. [import jshints list directly and combine them with the get-browser-globals.js] Improve dev tools to update browser variables list. Jul 19, 2020
@shapkarin
Copy link
Author

@sindresorhus summary:

  • jshint fork repo was imported as a devDependencies.
  • this PR contains some Improve for dev tools to update the browser variables list. Not sure that it's not WIP.
  • the same approach can be used for other environments.
  • this PR contains browser variables list.

@sindresorhus
Copy link
Owner

Again, we don't want to fetch from JSHint. The globals there are outdated. The whole point of this repo is to be the source of truth.

@shapkarin
Copy link
Author

OK, I got that. Maybe the reason that they want to keep all possible legacy support, not sure. I will close PR or later find some time to change the PR. Also it will be nice to change the README.md because it has some difference with yours response and create a misunderstanding.

@sindresorhus
Copy link
Owner

I would be happy to make the readme clearer, but I don't know what part is unclear.

@shapkarin
Copy link
Author

Extracted from JSHint and ESLint and merged.

@shapkarin
Copy link
Author

I can try to make it more clear maybe too. When I will back to my PR. Also plan to compare it with the other one.

@shapkarin
Copy link
Author

Extracted from JSHint and ESLint and merged.

I decide that it's a normal process – extract it every time when you need to update + yours tool for browser vars.

Also please check that file without that change I got an error at tests.

@sindresorhus
Copy link
Owner

Closing for lack of activity and also because this is not the solution we wanted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants