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

[WIP] Upgrade to TS 3.0 #5030

Merged
merged 1 commit into from Sep 14, 2018
Merged

[WIP] Upgrade to TS 3.0 #5030

merged 1 commit into from Sep 14, 2018

Conversation

blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 2, 2018

Takes advantage of the new project references feature to simplify how we build our packages.

Everything Just Works ™️ in that each folder gets its own lib/ as if it were built directly, huzzah! However, because the output of metapackage itself has changed, we need to update the way we build our API docs. We need to build docs in each folder and make our own index page. No code has to change, and Typedoc uses its own TypeScript compiler version, so we're good to go. This will actually be an improvement since our current API landing page is awkward.

  • Use project references
  • Update our integrity script to populate references
  • Clean up watch mode in light of these simplifications
  • Fix handling of update:dependency foo@next
  • Audit all watch scripts - do not use -b in examples since they should be standalone
  • Clean up API docs handling
  • Add top level landing page

Updated status: still blocked on a TypeScript Issue: microsoft/TypeScript#26955

@blink1073 blink1073 added this to the 0.34 milestone Aug 2, 2018
@SimonBiggs
Copy link
Member

Ahh this is awesome. Thanks @blink1073!

@jasongrout
Copy link
Contributor

Awesome, thanks!

I'm thinking we should maybe abandon Typedoc. It seems the project has stalled, and it is preventing us from using newer Typescript syntax (there are a few places noted in the code that we'd like to use new syntax).

One question is: how much are the api docs being used? Perhaps we should add a google analytics tracker to give us an idea. (plus it might be interesting to see which api pages are most used...)

@jasongrout
Copy link
Contributor

This was way easier than I thought. I thought we would be adding a references section to each package, keeping it in sync with the dependencies for the packages. We might eventually do that, but for now this makes the transition nice and smooth.

@SimonBiggs
Copy link
Member

@jasongrout if by typedoc you mean the following:

https://jupyterlab.github.io/jupyterlab/globals.html

I can let you know that personally I initially found it quite confusing, and in the end I found following definitions within visual studio code and reading source code to be easier.

@jasongrout
Copy link
Contributor

if by typedoc you mean the following:

https://jupyterlab.github.io/jupyterlab/globals.html

Yes, that's what I mean. Thanks for your feedback!

@blink1073
Copy link
Member Author

@SimonBiggs, was it confusing because of the way it was structured? If we had a better way to get at a class definition page, would that help?

@SimonBiggs
Copy link
Member

I guess traversal through the code base by using control + click within
visual studio code allowed me to quickly work out what was going on.
Everything was commented so well and in a consistent manner (I suspect for
use within typedoc) that all the information to use the API was available
there. If I ever needed more help I could jump out of the type definition
and go to the source code without much extra effort.

Being able to navigate that way, directly as I am writing code, is hard to
beat.

When I was new to the code base, typedoc was the only online documentation at
the time. Given that it was my first port of call to learn JupyterLab it
was overwhelming. The experience for new commers now might be quite a bit
different given that they would use the primary online docs first, and once
they are ready, might then use the API docs.

Personally I found, to understand what was really going on initially, I
really had to read through examples of how the API was being used within
the source code. I would find something I wanted to use, then run a search
for that method within the source code and be provided with examples of you
guys using it. I would sometimes just pull out my phone and scout through
the source code on GitHub on my phone. What I could learn by doing that was
far beyond what I felt I could learn looking at the typedoc.

@blink1073
Copy link
Member Author

I myself have never used the TypeDoc output for anything, preferring to read on Github or in Sublime Text as well. It has been a royal pain to maintain...

@blink1073
Copy link
Member Author

Oh no! I thought that jlpm run clean got rid of the lib/ folders, but it does not. If I remove those, then then whole thing falls apart.

../application-extension/lib/index.d.ts:1:34 - error TS2307: Cannot find module '@jupyterlab/application'.

1 import { JupyterLabPlugin } from '@jupyterlab/application';
                                   ~~~~~~~~~~~~~~~~~~~~~~~~~

I get that even when I add references to application-extension...

@blink1073
Copy link
Member Author

Oh wait, I made a mistake in specifying the references option.

@blink1073
Copy link
Member Author

Okay, this will work, but we do need to do what you thought @jasongrout, maintain the appropriate references for each project. That will be a part of the integrity check. The bonus thing is, you can then run yarn build from any package at any time, and it will Just Work ™️.

@jasongrout
Copy link
Contributor

yay for incremental compilation! I suppose --watch will work nicely as well?

@blink1073
Copy link
Member Author

It sounds that way, I won't know for sure until build works.

@blink1073
Copy link
Member Author

Drat, we need to wait on this problem in watch mode. Otherwise, it works great!

@blink1073
Copy link
Member Author

It takes ~1s to build on my machine when there are no changes, and ~73s from scratch.

@blink1073
Copy link
Member Author

But the amazing thing is that you can run yarn build from any package and it will Just Work ™️

@saulshanabrook
Copy link
Member

Yay, this is wonderful! thanks so much for working on this.

I am in favor of removing typedoc. I have never used it while getting acquainted with the codebase and like @SimonBiggs found it much easier to hop around it using vscode.

@saulshanabrook
Copy link
Member

Am I understanding things right that tsconfig.json references should match package.json references to other jupyterlab packages? I know that typescript has an issue for making it easier to have the references be inferred from other files, but in the meantime is there any checking to make sure those stay in sync?

"compilerOptions": {
"outDir": "../build"
},
"include": ["*"],
Copy link
Member

Choose a reason for hiding this comment

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

Can these "include": ["*"], options be moved to tsconfigbase?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, I tried and the files don't get built.

Copy link
Contributor

@jasongrout jasongrout Aug 4, 2018

Choose a reason for hiding this comment

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

All paths are relative to the tsconfig file they are in. So anything giving a path has to be in the individual tsconfig files.

Source: I also tried when making the tsconfigbase, and I also read the docs :)

@blink1073
Copy link
Member Author

The references are populated in our ensure-package script that is run in the integrity script.

@blink1073
Copy link
Member Author

Here are all the times we reference a class definition in our docs:

$ search "http://jupyterlab.github.io/jupyterlab/classes"
./developer/extension_dev.rst:91:`shell <http://jupyterlab.github.io/jupyterlab/classes/_application_src_shell_.applicationshell.html>`__
./developer/documents.rst:25:`Document widgets <http://jupyterlab.github.io/jupyterlab/classes/_docregistry_src_registry_.documentregistry.html>`__ represent a view of a document model. There can be multiple document widgets associated with a single document model, and they naturally stay in sync with each other since they are views on the same underlying data model.
./developer/documents.rst:29:Registry <http://jupyterlab.github.io/jupyterlab/classes/_docregistry_src_registry_.documentregistry.html>`__
./developer/documents.rst:35:Manager <http://jupyterlab.github.io/jupyterlab/classes/_docmanager_src_manager_.documentmanager.html>`__
./developer/documents.rst:51:`Widget Factories <http://jupyterlab.github.io/jupyterlab/classes/_docregistry_src_registry_.documentregistry.html#addwidgetfactory>`__
./developer/documents.rst:60:`Model Factories <http://jupyterlab.github.io/jupyterlab/classes/_docregistry_src_registry_.documentregistry.html#addmodelfactory>`__
./developer/documents.rst:68:`Widget Extension Factories <http://jupyterlab.github.io/jupyterlab/classes/_docregistry_src_registry_.documentregistry.html#addwidgetextension>`__
./developer/documents.rst:80:`File Types <http://jupyterlab.github.io/jupyterlab/classes/_docregistry_src_registry_.documentregistry.html#addfiletype>`__
./developer/documents.rst:86:`File Creators <http://jupyterlab.github.io/jupyterlab/classes/_docregistry_src_registry_.documentregistry.html>`__
./developer/notebook.rst:22:`NotebookWidgetFactory <http://jupyterlab.github.io/jupyterlab/classes/_notebook_src_widgetfactory_.notebookwidgetfactory.html>`__
./developer/notebook.rst:24:`NotebookPanel <http://jupyterlab.github.io/jupyterlab/classes/_notebook_src_panel_.notebookpanel.html>`__
./developer/notebook.rst:37:`NotebookModel <http://jupyterlab.github.io/jupyterlab/classes/_notebook_src_model_.notebookmodel.html>`__
./developer/notebook.rst:82:   widget <http://jupyterlab.github.io/jupyterlab/classes/_notebook_src_widget_.notebook.html>`__.
./developer/notebook.rst:99:`NotebookActions <http://jupyterlab.github.io/jupyterlab/classes/_notebook_src_actions_.notebookactions.html>`__
./developer/notebook.rst:112:   `InputArea <http://jupyterlab.github.io/jupyterlab/classes/_cells_src_inputarea_.inputarea.html>`__,
./developer/notebook.rst:115:      `CodeEditorWrapper <http://jupyterlab.github.io/jupyterlab/classes/_codeeditor_src_widget_.codeeditorwrapper.html>`__,
./developer/notebook.rst:120:`CodeCell <http://jupyterlab.github.io/jupyterlab/classes/_cells_src_widget_.codecell.html>`__
./developer/notebook.rst:122:`OutputArea <http://jupyterlab.github.io/jupyterlab/classes/_outputarea_src_widget_.outputarea.html>`__.
./developer/notebook.rst:124:`OutputAreaModel <http://jupyterlab.github.io/jupyterlab/classes/_outputarea_src_model_.outputareamodel.html>`__
./developer/notebook.rst:126:`RenderMimeRegistry <http://jupyterlab.github.io/jupyterlab/classes/_rendermime_src_registry_.rendermimeregistry.html>`__
./developer/notebook.rst:266:Registry <http://jupyterlab.github.io/jupyterlab/classes/_docregistry_src_registry_.documentregistry.html>`__.

My vote is to keep TypeDoc for now. There is an open (passing) PR to upgrade to 3.0: https://github.com/TypeStrong/typedoc/pull/825/files.

@SimonBiggs
Copy link
Member

SimonBiggs commented Aug 4, 2018 via email

@jasongrout
Copy link
Contributor

jasongrout commented Aug 4, 2018

My vote is to keep TypeDoc for now. There is an open (passing) PR to upgrade to 3.0: https://github.com/TypeStrong/typedoc/pull/825/files.

That was fast. There have been open PRs updating typedoc to other TS versions in the past months that went nowhere, though, so I'm not exactly holding my breath.

@blink1073
Copy link
Member Author

I'm not one to sit on my hands 😉. TypeStrong/typedoc#826 (comment)

@ellisonbg
Copy link
Contributor

Are we still going to try to get this in for 0.34?

@blink1073
Copy link
Member Author

We're blocked on fix in TypeScript. I'll leave this marked as 0.34 in case the fix is available in time.

@blink1073 blink1073 modified the milestones: 0.34, 0.35 Aug 13, 2018
@blink1073
Copy link
Member Author

Update: the fix to TypeScript is done in master and will be backported to 3.0.2.

@jasongrout
Copy link
Contributor

For reference, the TS issue was microsoft/TypeScript#26147, and the PRs fixing it are linked in that issue.

@blink1073
Copy link
Member Author

Update: we are no longer blocked! I'll return to this after #5251.

[04:37:29] Starting compilation in watch mode...

[04:37:29] Found 0 errors. Watching for file changes.

@blink1073
Copy link
Member Author

@jasongrout, why was --listEmittedFiles needed? I see you added it in #4703, but I can't puzzle out the reason it was needed. I ask because tsc --build does not support that flag. Also, we are blocked again by microsoft/TypeScript#26955.

@blink1073
Copy link
Member Author

If I use typescript@next, jupyter lab --dev-mode --watch is working again!

@blink1073
Copy link
Member Author

Okay, this is all done pending microsoft/TypeScript#26955.

@blink1073
Copy link
Member Author

Rock: meet hard place: TypeStrong/ts-loader#815. I'll remove --build support in our tests for now, until we finish the Jest conversion...

@blink1073 blink1073 self-assigned this Sep 12, 2018
wip convert to typescript 3.0

Add references field

wip ts 3.0

wip ts 3.0

wip ts 3.0

wip ts 3.0

wip ts 3.0

remove debug print

update to ts 3.0.3

Update to ts 3.0.3

Use webpack-env types properly

Clean up watch mode for metapackage

Use typescript@next for now

WIP update api docs building

WIP update api docs building

Fix handling of version in update-dependency

lint

wip cleanup

Remove references from examples

Clean up test watch:src scripts

Build all the docs

Add tdoptions files

add missing files

cleanup

formatting

Switch to ts 3.0.3 for now

Handle documentation index

Do not use references in examples

formatting

Update tests

lint

Add a header

Remove build compile from tests

lint

Fix composite

lint

lint updates

lint updates

prettier?

prettier?

Now prettier

No integrity

Try integrity again

Add prettier-changed files

cleanup

Clean up sibling scripts

fix webpack config

Fix docs runner

cleaup

Update handling of integrity

Cleanup

fix build utils

Fix integrity and theme building

Only add references if a src folder exists

Fix handling of targets check

Fix testutils

formatting

fix docs building

Add missing types

Add node types for tdoptions as well

clean up vega config

Fix typings
.lintstagedrc Show resolved Hide resolved
@jasongrout
Copy link
Contributor

@jasongrout, why was --listEmittedFiles needed?

I'm not sure if it was needed, though I found it a nice confirmation - if I switch to the watch window, I can confirm that the file I was working on was explicitly built. If it's messing up things, feel free to take it back off.

@blink1073
Copy link
Member Author

There's an open issue to add support for the rest of the flags for -w when using -b, but it isn't there yet. microsoft/TypeScript#25613

@blink1073 blink1073 merged commit 282ee23 into jupyterlab:master Sep 14, 2018
@blink1073 blink1073 mentioned this pull request Sep 14, 2018
@jasongrout jasongrout modified the milestones: 1.0, 0.35 Oct 1, 2018
@lock lock bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Aug 8, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
maintenance status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. tag:Build System
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants