[DYN-2316] feature: consume librarie.js as npm package #15228

Enzo707 · 2024-05-17T18:08:22Z

Purpose

This PR implements librarie.js consuming as npm package
Ref.: DYN-2316

Declarations

Check these if you believe they are true

The codebase is in a better state after this PR
Is documented according to the standards
The level of testing this PR includes is appropriate
User facing strings, if any, are extracted into *.resx files
All tests pass using the self-service CI.
Snapshot of UI changes, if any.
Changes to the API follow Semantic Versioning and are documented in the API Changes document.
This PR modifies some build requirements and the readme is updated
This PR contains no files larger than 50 MB

Reviewers

@QilongTang
@RobertGlobant20

FYIs

@mjkkirschner

github-actions

See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-2316

QilongTang · 2024-05-17T18:21:02Z

src/LibraryViewExtensionWebView2/web/library/build/librarie.min.js.LICENSE.txt

@@ -0,0 +1,45 @@
+/*


hmm this file needs to be hard copied over?

we can removed it.

github-actions · 2024-05-17T18:28:00Z

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net8.0

RobertGlobant20

I have some minor comments and some comments about the files included in this PR.

RobertGlobant20 · 2024-05-17T18:35:00Z

src/LibraryViewExtensionWebView2/LibraryViewExtensionWebView2.csproj

+        </PropertyGroup>
+
+        <!--Extracts the file to /package-->
+        <Exec Command="tar -xzf $(MSBuildProjectDirectory)\$(Last).tgz --strip-components=1 --directory=web/library"></Exec>


are not following the same structure than for the other packages? something like Packages/LibrarieJS

src/LibraryViewExtensionWebView2/LibraryViewController.cs

RobertGlobant20 · 2024-05-17T18:44:11Z

src/LibraryViewExtensionWebView2/web/library/build/librarie.min.js.LICENSE.txt

@@ -0,0 +1,45 @@
+/*


you should NOT include this files as part of your PR:

librarie.min.js

librarie.min.js.LICENSE.txt

librarie.min.js.map
it's is supposed that when the LibraryViewExtensionWebView2 project is build it will be downloading the package, unzip it ...... then this files will be present at that time, am i right?.

I've seen librarie.min.js needs to be included (I suppose as it is an EmbeddedResource ) otherwise we'll get an error in Library controller related to StreamReader. @RobertGlobant20 do you know why this might be happening?

@Enzo707 have you resolved this or do you still need unblocking here?

RobertGlobant20 · 2024-05-17T18:45:52Z

src/LibraryViewExtensionWebView2/LibraryViewController.cs

@@ -287,23 +287,23 @@ private string ReplaceUrlWithBase64Image(string html, string minifiedURL, bool m
        //list of resources which have paths embedded directly into the source.
        private readonly Tuple<string, bool>[] dynamicResourcePaths = new Tuple<string, bool>[]
        {
-           Tuple.Create("/resources/ArtifaktElement-Bold.woff",true),


Why the "/" is not needed?

Why the "/" is not needed?

Because we changed the public path in package so the bundle has changed

src/LibraryViewExtensionWebView2/Packages/LibrarieJS/package.json

mjkkirschner · 2024-05-20T15:51:54Z

Why are you committing any of the npm package contents into the repo?

Ah, I see you and @RobertGlobant20 discussed a bit above - you should be able to make sure that the build pulls the npm package before it attempts to embed the resource.

I think that having a version of the min file committed and an npm package pulled will lead to confusing build results where the actual embedded version is unclear - that is exactly what we are trying to avoid by pulling a specific version of the package.

Enzo707 · 2024-05-20T16:13:30Z

Why are you committing any of the npm package contents into the repo?

Ah, I see you and @RobertGlobant20 discussed a bit above - you should be able to make sure that the build pulls the npm package before it attempts to embed the resource.

I think that having a version of the min file committed and an npm package pulled will lead to confusing build results where the actual embedded version is unclear - that is exactly what we are trying to avoid by pulling a specific version of the package.

Make sense, I've removed all hard copied files that will be installed by npm package.
I was getting an error trying to read the librarie.min.js resource within the Library controller. @RobertGlobant20 please let me know if you are able to reproduce it.

mjkkirschner · 2024-05-21T15:33:02Z

what's with the failing self serve job?

QilongTang · 2024-05-21T15:53:45Z

what's with the failing self serve job?

It timed out, I restarted it

QilongTang · 2024-05-21T17:34:09Z

One regression to look at @Enzo707
NodeDocumentationMarkdownGeneratorTests.MarkdownGeneratorCommandTests.DictionaryContentIsFoundCorrectlyForCoreNodes

QilongTang · 2024-05-22T01:18:40Z

One regression to look @Enzo707
DynamoCoreWpfTests.NodeAutoCompleteSearchTests.NodeSuggestions_CanAutoCompleteOnCustomNodesOutPort_WithWhiteSpaceStartingPortName

Enzo707 · 2024-05-22T19:02:44Z

One regression to look @Enzo707 DynamoCoreWpfTests.NodeAutoCompleteSearchTests.NodeSuggestions_CanAutoCompleteOnCustomNodesOutPort_WithWhiteSpaceStartingPortName

@QilongTang I've made a rebase master on this branch and all tests passed

…anvas (DynamoDS#15224)

…moDS#15225) Co-authored-by: Aaron (Qilong) <173288704@qq.com>

Co-authored-by: Trygve Wastvedt <trygve.wastvedt@autodesk.com> Co-authored-by: pinzart90 <tiberiu.pinzariu@gmail.com>

Co-authored-by: Trygve Wastvedt <trygve.wastvedt@gmail.com> Co-authored-by: Trygve Wastvedt <trygve.wastvedt@autodesk.com>

mjkkirschner · 2024-05-23T01:49:00Z

doc/distrib/NodeHelpFiles/CoreNodeModels.DefineData.md

@@ -1,23 +1,26 @@
 ## In Depth

-The Define Data node validates the data type of incoming data. It can be used to ensure local data is of the desired type and is also designed to be used as an input or output node, declaring the type of data a graph expects or provides. The node supports a selection of commonly used Dynamo data types, for example 'String', 'Point', or 'Boolean'. The full list of supported data types is available in the drop-down menu of the node.


seems like your PR is now touching unrelated files?

Or is this just showing commits that are already in master?

@mjkkirschner yep, those commits are in master already

QilongTang · 2024-05-23T17:58:44Z

@Enzo707 Does not feel rebase would be the best option here, next time maybe just merge the changes on master to your fork?

Enzo707 · 2024-05-23T19:03:04Z

@Enzo707 Does not feel rebase would be the best option here, next time maybe just merge the changes on master to your fork?

@QilongTang Got it!

feat: add librarie.js as npm package

8fc980b

github-actions bot reviewed May 17, 2024

View reviewed changes

QilongTang reviewed May 17, 2024

View reviewed changes

RobertGlobant20 reviewed May 17, 2024

View reviewed changes

feat: change file structure and remove hard copied files

7951981

QilongTang reviewed May 19, 2024

View reviewed changes

src/LibraryViewExtensionWebView2/Packages/LibrarieJS/package.json Outdated Show resolved Hide resolved

EnzoBatistaU added 2 commits May 20, 2024 10:29

remove package.json

e3df898

remove README

eeb50a4

remove librarie.min.js

d9861d4

QilongTang added this to the 3.2 milestone May 20, 2024

QilongTang approved these changes May 20, 2024

View reviewed changes

fix(MarkdownGeneratorCommandTests): regression

aa7dddb

pinzart90 and others added 8 commits May 22, 2024 16:07

fix extension assembly paths for linux (DynamoDS#15227)

926aab5

DYN-6967 Crash when placing List selector archi-lab Dynamo nodes in c…

4791c96

…anvas (DynamoDS#15224)

DYN-6693 Alert user for duplicate files when publishing package (Dyna…

da07709

…moDS#15225) Co-authored-by: Aaron (Qilong) <173288704@qq.com>

DYN-6427 & DYN-6828 Graph Properties UI Fixes (DynamoDS#15200)

e4eedfb

Co-authored-by: Trygve Wastvedt <trygve.wastvedt@autodesk.com> Co-authored-by: pinzart90 <tiberiu.pinzariu@gmail.com>

DYN-6990 Crash when selecting node with view extension (DynamoDS#15231)

5726921

Fix regression for message box (DynamoDS#15232)

c65fe77

DYN-6633: Definedata UI update (DynamoDS#15234)

032b96a

Co-authored-by: Trygve Wastvedt <trygve.wastvedt@gmail.com> Co-authored-by: Trygve Wastvedt <trygve.wastvedt@autodesk.com>

DYN-6633: Definedata bugfix heterogenous lists (DynamoDS#15235)

b50ce8b

Co-authored-by: Trygve Wastvedt <trygve.wastvedt@gmail.com> Co-authored-by: Trygve Wastvedt <trygve.wastvedt@autodesk.com>

mjkkirschner reviewed May 23, 2024

View reviewed changes

QilongTang merged commit e223c97 into DynamoDS:master May 23, 2024
24 checks passed

QilongTang mentioned this pull request May 23, 2024

[DYN-2316] Fix what's broken #15242

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DYN-2316] feature: consume librarie.js as npm package #15228

[DYN-2316] feature: consume librarie.js as npm package #15228

Enzo707 commented May 17, 2024 •

edited

github-actions bot left a comment

QilongTang May 17, 2024

Enzo707 May 17, 2024 •

edited

github-actions bot commented May 17, 2024 •

edited

RobertGlobant20 left a comment

RobertGlobant20 May 17, 2024

RobertGlobant20 May 17, 2024

Enzo707 May 17, 2024 •

edited

mjkkirschner May 23, 2024

RobertGlobant20 May 17, 2024

Enzo707 May 17, 2024 •

edited

mjkkirschner commented May 20, 2024 •

edited

Enzo707 commented May 20, 2024 •

edited

mjkkirschner commented May 21, 2024

QilongTang commented May 21, 2024

QilongTang commented May 21, 2024

QilongTang commented May 22, 2024

Enzo707 commented May 22, 2024

mjkkirschner May 23, 2024 •

edited

mjkkirschner May 23, 2024 •

edited

Enzo707 May 23, 2024

QilongTang commented May 23, 2024

Enzo707 commented May 23, 2024

		@@ -1,23 +1,26 @@
		## In Depth

		The Define Data node validates the data type of incoming data. It can be used to ensure local data is of the desired type and is also designed to be used as an input or output node, declaring the type of data a graph expects or provides. The node supports a selection of commonly used Dynamo data types, for example 'String', 'Point', or 'Boolean'. The full list of supported data types is available in the drop-down menu of the node.

[DYN-2316] feature: consume librarie.js as npm package #15228

[DYN-2316] feature: consume librarie.js as npm package #15228

Conversation

Enzo707 commented May 17, 2024 • edited

Purpose

Declarations

Reviewers

FYIs

github-actions bot left a comment

Choose a reason for hiding this comment

QilongTang May 17, 2024

Choose a reason for hiding this comment

Enzo707 May 17, 2024 • edited

Choose a reason for hiding this comment

github-actions bot commented May 17, 2024 • edited

UI Smoke Tests

RobertGlobant20 left a comment

Choose a reason for hiding this comment

RobertGlobant20 May 17, 2024

Choose a reason for hiding this comment

RobertGlobant20 May 17, 2024

Choose a reason for hiding this comment

Enzo707 May 17, 2024 • edited

Choose a reason for hiding this comment

mjkkirschner May 23, 2024

Choose a reason for hiding this comment

RobertGlobant20 May 17, 2024

Choose a reason for hiding this comment

Enzo707 May 17, 2024 • edited

Choose a reason for hiding this comment

mjkkirschner commented May 20, 2024 • edited

Enzo707 commented May 20, 2024 • edited

mjkkirschner commented May 21, 2024

QilongTang commented May 21, 2024

QilongTang commented May 21, 2024

QilongTang commented May 22, 2024

Enzo707 commented May 22, 2024

mjkkirschner May 23, 2024 • edited

Choose a reason for hiding this comment

mjkkirschner May 23, 2024 • edited

Choose a reason for hiding this comment

Enzo707 May 23, 2024

Choose a reason for hiding this comment

QilongTang commented May 23, 2024

Enzo707 commented May 23, 2024

Enzo707 commented May 17, 2024 •

edited

Enzo707 May 17, 2024 •

edited

github-actions bot commented May 17, 2024 •

edited

Enzo707 May 17, 2024 •

edited

Enzo707 May 17, 2024 •

edited

mjkkirschner commented May 20, 2024 •

edited

Enzo707 commented May 20, 2024 •

edited

mjkkirschner May 23, 2024 •

edited

mjkkirschner May 23, 2024 •

edited