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

[TS MVP] Update NPM release process #11720

Closed
arkflpc opened this issue May 6, 2022 · 5 comments
Closed

[TS MVP] Update NPM release process #11720

arkflpc opened this issue May 6, 2022 · 5 comments
Labels
domain:ts squad:core Issue to be handled by the Core team. type:task This issue reports a chore (non-production change) and other types of "todos".

Comments

@arkflpc
Copy link
Contributor

arkflpc commented May 6, 2022

We should be able to publish NPM packages from sources written in both JavaScript and TypeScript.

The open question is: how much backward compatibility we want to achieve? If the users import modules directly reaching src directories, should we continue to support that or ask them to import from dist (or any other name) directory instead?

Part of #11708.

@arkflpc arkflpc added squad:core Issue to be handled by the Core team. type:task This issue reports a chore (non-production change) and other types of "todos". domain:ts and removed type:task This issue reports a chore (non-production change) and other types of "todos". labels May 6, 2022
@arkflpc
Copy link
Contributor Author

arkflpc commented Jun 8, 2022

We should check if the online builder works with the new process or update it.
We should write migration guide if there are any incompatibilities.

@pomek
Copy link
Member

pomek commented Jun 30, 2022

First, we need to answer a question: what do we want to publish?

  • We must publish *.js files as we did before the migration as we don't want to break projects depending on our packages.
  • We should publish typings as the TypeScript compilator should not complain about missing typings when importing CKEditor 5 files.
  • We should not publish *.ts files as source files should not be modified by integrators in their node_modules/ directory.

We need to understand the current release path to identify the changes required in the release process.

  • Prepare a changelog containing the packages' names to be released.
  • Update the versions of packages to be released.
    • Bump version in package.json (using the npm version ... command)
    • Bump version of dependencies and devDependencies
  • Npm publish.
    • For each package, we call the npm publish command.

We can use pre/post hooks for attaching additional steps.

Transpiling TS to JS must be done before publishing packages but after bumping their versions.

Hence, I suggest adding the postversion hook, which allows doing the final check before publishing things.

I prepared on a separate branch a solution that plugs the compilation step right after bumping the version: ck/epic/11708-migrate-typescript-mvp...ck/11720-ts-publish

Also, I share the result of executing the npm pack command.
ckeditor-ckeditor5-utils-34.1.0.tgz

We can use the example package to check whether the TS compiler can handle the package. If not, we would need to identify problems and redesign the solution for fixing these.

@Reinmar
Copy link
Member

Reinmar commented Jul 4, 2022

Scenarios that I'd like to verify:

  • Custom plugin development. Importing stuff from our packages. Do typings work in the build process? Do typings work in IDEs (VSCode / PHPStorm)? We would have a nice playground to test this if our package generator had an option to output TS-based env.
  • Using a build (e.g. ckeditor5-build-classic). Do typings work in someone's build process / IDE? Do they work e.g. when using in Angular/React/Vue apps? Assuming that a build has just one d.ts file, does this file contain typings e.g. for ckeditor5-engine/src/model~Model? What happens if I access ( await ClassicEditor.create() ).model?

Questions:

  • Should online builder output an environment (its source) in TS or JS?
    • The answer is: TS. Otherwise, the output build could not contain typings easily.

@pomek
Copy link
Member

pomek commented Jul 11, 2022

TL;DR: The following steps should confirm that our builders (so far) can handle a TypeScript package published on npm.

Steps:

  1. CKE5: git fetch

  2. CKE5: git checkout ck/11720-ts-publish

  3. CKE5: yarn run reinstall

  4. CKE5: cd packages/ckeditor5-utils/

  5. yarn run build - it compiles TS to JS and typings

  6. You need to remove all source *.ts files. You don't have to remove typings.

    • Otherwise, webpack will import TypeScript when resolving paths.
  7. Replace the *.ts extension with *.js in the #main key package.json.

  8. cd ../../

  9. CKE5: yarn run manual -f build-classic

  10. Open http://localhost:8125/ckeditor5-build-classic/tests/manual/ckeditor.html and see the value of the CKEDITOR_VERSION variable. It should be 34.2.0.

  11. CKE5: git co master -- packages/ckeditor5-engine/src/**

    • Since ckeditor5-engine is partially written in TS, we need to checkout to full JS version.
  12. Create a simple plugin that will use a code from @ckeditor/ckeditor5-utils.

    import { toArray } from 'ckeditor5/src/utils';
    import { Plugin } from 'ckeditor5/src/core';
    
    class CustomPlugin extends Plugin {
        afterInit() {
      	  console.log( toArray() );
      	  // An IDE should report a warning:
      	  // "Invalid number of arguments, expected 1"
        }
    }
    
    export default class ClassicEditor extends ClassicEditorBase {
    }
    
    // Plugins to include in the build.
    ClassicEditor.builtinPlugins = [
        CustomPlugin,
  13. CKE5: cd packages/ckeditor5-build-classic

  14. yarn run build

    • It should add the CustomPlugin to the build.
    • As we don't use TS compiler (since we build the src/ckeditor.js file), it should not complain regarding the number of required arguments.
  15. Once it's done, refresh the manual test page. It should print the following console.log: [undefined].

  16. Now we can revert all changes we have done:

    • Close the manual test server.
    • git reset packages/ckeditor5-engine/ && git checkout packages/ckeditor5-engine/
    • git reset packages/ckeditor5-utils/ && git checkout packages/ckeditor5-utils/
    • git checkout packages/ckeditor5-build-classic/

@psmyrek
Copy link
Contributor

psmyrek commented Jul 11, 2022

  1. Once it's done, refresh the manual test page.

In my case, I had to restart the manual test server because changes were not detected automatically. Besides that, the other points work as described.

@CKEditorBot CKEditorBot added the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Jul 13, 2022
arkflpc added a commit that referenced this issue Jul 14, 2022
Internal: Prepared the repository to release a package written in TypeScript. Closes #11720.
@arkflpc arkflpc closed this as completed Jul 18, 2022
@CKEditorBot CKEditorBot removed the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Jul 18, 2022
@CKEditorBot CKEditorBot added this to the upcoming milestone Jul 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:ts squad:core Issue to be handled by the Core team. type:task This issue reports a chore (non-production change) and other types of "todos".
Projects
None yet
Development

No branches or pull requests

5 participants