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

Move off node10 and add intellisense #4643

Merged
merged 21 commits into from
Jan 7, 2022
Merged

Move off node10 and add intellisense #4643

merged 21 commits into from
Jan 7, 2022

Conversation

unlikelyzero
Copy link
Contributor

@unlikelyzero unlikelyzero commented Dec 30, 2021

Closes: #4584 and #4690

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Is this change backwards compatible? For example, developers won't need to change how they are calling the API or how they've extended core plugins such as Tables or Plots.

Author Checklist

  • Changes address original issue?
  • Unit tests included and/or updated with changes?
  • Command line build passes?
  • Has this been smoke tested?
  • Testing instructions included in associated issue?

@unlikelyzero unlikelyzero linked an issue Dec 30, 2021 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Dec 30, 2021

Codecov Report

Merging #4643 (2b26987) into master (f6934a4) will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4643      +/-   ##
==========================================
+ Coverage   56.37%   56.41%   +0.03%     
==========================================
  Files         718      718              
  Lines       22807    22807              
  Branches     1658     1658              
==========================================
+ Hits        12857    12866       +9     
+ Misses       9539     9533       -6     
+ Partials      411      408       -3     
Impacted Files Coverage Δ
src/api/composition/DefaultCompositionProvider.js 77.27% <0.00%> (-4.55%) ⬇️
src/plugins/imagery/components/ImageryTimeView.vue 72.00% <0.00%> (-0.58%) ⬇️
...c/plugins/persistence/couch/CouchObjectProvider.js 81.85% <0.00%> (-0.45%) ⬇️
src/ui/components/ObjectView.vue 45.10% <0.00%> (+1.08%) ⬆️
src/ui/inspector/Inspector.vue 75.00% <0.00%> (+8.33%) ⬆️
src/ui/inspector/details/Properties.vue 78.18% <0.00%> (+12.72%) ⬆️
src/tools/url.js 100.00% <0.00%> (+30.76%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f6934a4...2b26987. Read the comment docs.

shefalijoshi
shefalijoshi previously approved these changes Dec 30, 2021
Copy link
Contributor

@shefalijoshi shefalijoshi left a comment

Choose a reason for hiding this comment

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

LGTM

@shefalijoshi
Copy link
Contributor

@unlikelyzero Tests are failing.

@unlikelyzero
Copy link
Contributor Author

unlikelyzero commented Dec 30, 2021 via email

unlikelyzero and others added 2 commits January 3, 2022 13:49
…ntial issue with Node 17 in case the config changes closer to defaults

At the moment there is no error with Node 17 because the config strays from the defaults and avoids the common case.

Also add a tsconfig.json file that enables VS Code and other IDEs to perform type checking on the side. For example now the webpack config file is type checked. This does not impact any existing processes, our build scripts are left untouched and only IDEs will use it for live intellisense and type checking when viewing files (f.e. showing helpful red squiggly underlines on type errors)
@unlikelyzero
Copy link
Contributor Author

@trusktr that change still didn't take on node16. It may only apply to node17?

@unlikelyzero unlikelyzero requested a review from trusktr January 5, 2022 00:39
@trusktr
Copy link
Contributor

trusktr commented Jan 5, 2022

@trusktr that change still didn't take on node16. It may only apply to node17?

True, that was only for Node 17. Trying to find out this Node 16 mystery. Gonna start toggling settings in webpack conf.

@unlikelyzero unlikelyzero changed the title Move off node10 and bring in node16 Move off node10 Jan 6, 2022
@unlikelyzero
Copy link
Contributor Author

@shefalijoshi when you have a chance can you look at the ts config?

@unlikelyzero unlikelyzero changed the title Move off node10 Move off node10 and add intellisense Jan 7, 2022
@unlikelyzero unlikelyzero linked an issue Jan 7, 2022 that may be closed by this pull request
@unlikelyzero unlikelyzero requested a review from akhenry January 7, 2022 18:09
@unlikelyzero unlikelyzero merged commit b3ab56c into master Jan 7, 2022
@unlikelyzero unlikelyzero deleted the node16-or-bust branch January 7, 2022 19:39
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.

[Build] Support IDE Intellisense [Build] Move off of node10
4 participants